login: Allow explicit disabling of site isolation
When site isolation device policies are set to a value that should
disable site isolation (DeviceLoginScreenSitePerProcess is false, or
DeviceLoginScreenIsolateOrigins is empty), pass an additional flag to
prevent trials from enabling site isolation.
Note: Please see also the respective user policy evaluation change on
the chrome side in CL:997737.
BUG=chromium:827821
TEST=cros_run_unit_tests --board=${BOARD} --packages \
chromeos-base/chromeos-login
Change-Id: I286d0187ff79dc60e463a2971430cff4620c4cd8
Reviewed-on: https://chromium-review.googlesource.com/1005155
Commit-Ready: Pavol Marko <pmarko@chromium.org>
Tested-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit eadfad93b548d2dad2ed0d8d4ec96ff1f0fa9d44)
Reviewed-on: https://chromium-review.googlesource.com/1012602
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Josafat Garcia <josafat@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Commit-Queue: Josafat Garcia <josafat@chromium.org>
Tested-by: Charlie Reis <creis@chromium.org>
Tested-by: Josafat Garcia <josafat@chromium.org>
Trybot-Ready: Josafat Garcia <josafat@chromium.org>
diff --git a/login_manager/device_policy_service.cc b/login_manager/device_policy_service.cc
index af65849..7a032be 100644
--- a/login_manager/device_policy_service.cc
+++ b/login_manager/device_policy_service.cc
@@ -325,22 +325,41 @@
}
}
+ // A enterprise-managed DeviceLoginScreenSitePerProcess policy which is set
+ // to false, or an enterprise-managed DeviceLoginScreenIsolateOrigins policy
+ // which is set to an empty string are treated as explicitly disabling the
+ // respective site isolation feature.
+ // To ensure this is the case, a flag will be added to also disable site
+ // isolation trials.
+ bool disable_site_isolation_trials = false;
+
// Respect DeviceLoginScreenSitePerProcess policy for the sign-in screen.
if (policy.has_device_login_screen_site_per_process()) {
const em::DeviceLoginScreenSitePerProcessProto& proto =
policy.device_login_screen_site_per_process();
- if (proto.has_site_per_process())
- policy_args.push_back("--site-per-process");
+ if (proto.has_site_per_process()) {
+ if (proto.site_per_process())
+ policy_args.push_back("--site-per-process");
+ else
+ disable_site_isolation_trials = true;
+ }
}
// Respect DeviceLoginScreenIsolateOrigins for the sign-in screen.
if (policy.has_device_login_screen_isolate_origins()) {
const em::DeviceLoginScreenIsolateOriginsProto& proto =
policy.device_login_screen_isolate_origins();
- if (proto.has_isolate_origins())
- policy_args.push_back("--isolate-origins=" + proto.isolate_origins());
+ if (proto.has_isolate_origins()) {
+ if (!proto.isolate_origins().empty())
+ policy_args.push_back("--isolate-origins=" + proto.isolate_origins());
+ else
+ disable_site_isolation_trials = true;
+ }
}
+ if (disable_site_isolation_trials)
+ policy_args.push_back("--disable-site-isolation-trials");
+
// Add sentinel values to mark which flags were filled from policy and should
// not apply to user sessions.
if (!policy_args.empty()) {
diff --git a/login_manager/device_policy_service_unittest.cc b/login_manager/device_policy_service_unittest.cc
index 1a85021..090e1e2 100644
--- a/login_manager/device_policy_service_unittest.cc
+++ b/login_manager/device_policy_service_unittest.cc
@@ -1043,6 +1043,33 @@
"--site-per-process", "--policy-switches-end"));
}
+TEST_F(DevicePolicyServiceTest, StartUpFlagsSitePerProcessDisabled) {
+ MockNssUtil nss;
+ InitService(&nss, true);
+
+ em::ChromeDeviceSettingsProto settings;
+ // Explicitly disable DeviceLoginScreenSitePerProcess.
+ settings.mutable_device_login_screen_site_per_process()->set_site_per_process(
+ false);
+ ASSERT_NO_FATAL_FAILURE(InitPolicy(settings, owner_, fake_sig_, ""));
+ SetExpectationsAndStorePolicy(MakeChromePolicyNamespace(), store_,
+ policy_proto_);
+ EXPECT_THAT(
+ service_->GetStartUpFlags(),
+ ElementsAre("--policy-switches-begin", "--disable-site-isolation-trials",
+ "--policy-switches-end"));
+
+ // Additional policy-set arbitrary flags
+ settings.mutable_start_up_flags()->add_flags("--a");
+ ASSERT_NO_FATAL_FAILURE(InitPolicy(settings, owner_, fake_sig_, ""));
+ SetExpectationsAndStorePolicy(MakeChromePolicyNamespace(), store_,
+ policy_proto_);
+ EXPECT_THAT(
+ service_->GetStartUpFlags(),
+ ElementsAre("--policy-switches-begin", "--a",
+ "--disable-site-isolation-trials", "--policy-switches-end"));
+}
+
TEST_F(DevicePolicyServiceTest, StartUpFlagsIsolateOrigins) {
MockNssUtil nss;
InitService(&nss, true);
@@ -1079,6 +1106,33 @@
"--policy-switches-end"));
}
+TEST_F(DevicePolicyServiceTest, StartUpFlagsIsolateOriginsDisabled) {
+ MockNssUtil nss;
+ InitService(&nss, true);
+
+ em::ChromeDeviceSettingsProto settings;
+ // Explicitly disable DeviceLoginScreenIsolateOrigins.
+ settings.mutable_device_login_screen_isolate_origins()->set_isolate_origins(
+ "");
+ ASSERT_NO_FATAL_FAILURE(InitPolicy(settings, owner_, fake_sig_, ""));
+ SetExpectationsAndStorePolicy(MakeChromePolicyNamespace(), store_,
+ policy_proto_);
+ EXPECT_THAT(
+ service_->GetStartUpFlags(),
+ ElementsAre("--policy-switches-begin", "--disable-site-isolation-trials",
+ "--policy-switches-end"));
+
+ // --isolate-origins and policy-set arbitrary flags
+ settings.mutable_start_up_flags()->add_flags("--a");
+ ASSERT_NO_FATAL_FAILURE(InitPolicy(settings, owner_, fake_sig_, ""));
+ SetExpectationsAndStorePolicy(MakeChromePolicyNamespace(), store_,
+ policy_proto_);
+ EXPECT_THAT(
+ service_->GetStartUpFlags(),
+ ElementsAre("--policy-switches-begin", "--a",
+ "--disable-site-isolation-trials", "--policy-switches-end"));
+}
+
TEST_F(DevicePolicyServiceTest, PersistPolicyMultipleNamespaces) {
MockNssUtil nss;
InitService(&nss, true);
