Commit 11265cda authored by Aya ElAttar's avatar Aya ElAttar Committed by Commit Bot

Fix safe permissions triggering full warning

Fixed a reversed condition caused safe API permissions trigger
full warning on the login screen of the managed-guest session.


Bug: 1096838
Change-Id: I535d823c26de93a6934e82e61c2b12b27fc73680
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2253758Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarSergey Poromov <poromov@chromium.org>
Commit-Queue: Aya Elsayed <ayaelattar@google.com>
Cr-Commit-Position: refs/heads/master@{#780471}
parent 5852bae1
...@@ -75,14 +75,14 @@ bool ExtensionsPermissionsTracker::IsSafePerms( ...@@ -75,14 +75,14 @@ bool ExtensionsPermissionsTracker::IsSafePerms(
const PermissionSet& active_permissions = perms_data->active_permissions(); const PermissionSet& active_permissions = perms_data->active_permissions();
const APIPermissionSet& api_permissions = active_permissions.apis(); const APIPermissionSet& api_permissions = active_permissions.apis();
for (auto* permission : api_permissions) { for (auto* permission : api_permissions) {
if (!permission->info()->requires_managed_session_full_login_warning()) { if (permission->info()->requires_managed_session_full_login_warning()) {
return false; return false;
} }
} }
const ManifestPermissionSet& manifest_permissions = const ManifestPermissionSet& manifest_permissions =
active_permissions.manifest_permissions(); active_permissions.manifest_permissions();
for (const auto* permission : manifest_permissions) { for (const auto* permission : manifest_permissions) {
if (!permission->RequiresManagedSessionFullLoginWarning()) { if (permission->RequiresManagedSessionFullLoginWarning()) {
return false; return false;
} }
} }
......
...@@ -2246,6 +2246,9 @@ class ManagedSessionsTest : public DeviceLocalAccountTest { ...@@ -2246,6 +2246,9 @@ class ManagedSessionsTest : public DeviceLocalAccountTest {
kGoodExtensionID, kGoodExtensionVersion, kGoodExtensionID, kGoodExtensionVersion,
embedded_test_server()->GetURL(std::string("/") + embedded_test_server()->GetURL(std::string("/") +
kGoodExtensionCRXPath)); kGoodExtensionCRXPath));
testing_update_manifest_provider->AddUpdate(
kHostedAppID, kHostedAppVersion,
embedded_test_server()->GetURL(std::string("/") + kHostedAppCRXPath));
testing_update_manifest_provider->AddUpdate( testing_update_manifest_provider->AddUpdate(
kShowManagedStorageID, kShowManagedStorageVersion, kShowManagedStorageID, kShowManagedStorageVersion,
embedded_test_server()->GetURL(std::string("/") + embedded_test_server()->GetURL(std::string("/") +
...@@ -2273,7 +2276,15 @@ class ManagedSessionsTest : public DeviceLocalAccountTest { ...@@ -2273,7 +2276,15 @@ class ManagedSessionsTest : public DeviceLocalAccountTest {
embedded_test_server()->GetURL(kRelativeUpdateURL).spec().c_str())); embedded_test_server()->GetURL(kRelativeUpdateURL).spec().c_str()));
} }
void AddForceInstalledExtension() { AddExtension(kGoodExtensionID); } void AddForceInstalledSafeExtension() { AddExtension(kHostedAppID); }
void AddForceInstalledUnsafeExtension() {
// has effective hosts:
// http://*.example.com/*
// http://*.google.com/*
// https://*.google.com/*
AddExtension(kGoodExtensionID);
}
void AddForceInstalledWhitelistedExtension() { void AddForceInstalledWhitelistedExtension() {
AddExtension(kShowManagedStorageID); AddExtension(kShowManagedStorageID);
...@@ -2364,10 +2375,54 @@ IN_PROC_BROWSER_TEST_F(ManagedSessionsTest, ManagedSessionsEnabledNonRisky) { ...@@ -2364,10 +2375,54 @@ IN_PROC_BROWSER_TEST_F(ManagedSessionsTest, ManagedSessionsEnabledNonRisky) {
broker)); broker));
} }
IN_PROC_BROWSER_TEST_F(ManagedSessionsTest, ForceInstalledExtension) { IN_PROC_BROWSER_TEST_F(ManagedSessionsTest, ForceInstalledSafeExtension) {
SetManagedSessionsEnabled(/* managed_sessions_enabled */ true);
StartTestExtensionsServer();
AddForceInstalledSafeExtension();
// Install and refresh the device policy now. This will also fetch the initial
// user policy for the device-local account now.
UploadAndInstallDeviceLocalAccountPolicy();
AddPublicSessionToDevicePolicy(kAccountId1);
WaitForPolicy();
const user_manager::User* user =
user_manager::UserManager::Get()->FindUser(account_id_1_);
ASSERT_TRUE(user);
auto* broker = GetDeviceLocalAccountPolicyBroker();
ASSERT_TRUE(broker);
// Check that 'DeviceLocalAccountManagedSessionEnabled' policy was applied
// correctly.
EXPECT_TRUE(
chromeos::ChromeUserManager::Get()->IsManagedSessionEnabledForUser(
*user));
// Management disclosure warning is shown in the beginning, because
// kManagedSessionUseFullLoginWarning pref is set to true in the beginning.
ASSERT_TRUE(
chromeos::ChromeUserManager::Get()->IsFullManagementDisclosureNeeded(
broker));
ExtensionInstallObserver install_observer(kHostedAppID);
ASSERT_NO_FATAL_FAILURE(StartLogin(std::string(), std::string()));
WaitForSessionStart();
install_observer.Wait();
// After the login, kManagedSessionUseFullLoginWarning pref is updated.
// Check that force-installed extension activates managed session mode for
// device-local users.
EXPECT_FALSE(
chromeos::ChromeUserManager::Get()->IsFullManagementDisclosureNeeded(
broker));
}
IN_PROC_BROWSER_TEST_F(ManagedSessionsTest, ForceInstalledUnsafeExtension) {
SetManagedSessionsEnabled(/* managed_sessions_enabled */ true); SetManagedSessionsEnabled(/* managed_sessions_enabled */ true);
StartTestExtensionsServer(); StartTestExtensionsServer();
AddForceInstalledExtension(); AddForceInstalledUnsafeExtension();
// Install and refresh the device policy now. This will also fetch the initial // Install and refresh the device policy now. This will also fetch the initial
// user policy for the device-local account now. // user policy for the device-local account now.
......
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