Skip to content

Commit

Permalink
Make extensions developer mode adhere to policy
Browse files Browse the repository at this point in the history 
Make the developer mode checkbox on chrome://extensions adhere to the DeveloperToolsDisabled policy. If it is disabled, display an indicator.
Add corresponding indicator test to policy_prefs_browsertest.
Add an end-to-end test to policy_browsertest.

BUG=633684
TEST=browser_tests *PolicyPrefIndicatorTest*, PolicyTest.DeveloperToolsDisabledExtensionsDevMode
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2529083002
Cr-Commit-Position: refs/heads/master@{#438215}
  • Loading branch information
pmarko authored and Commit bot committed Dec 13, 2016
1 parent a40ee20 commit 6e36b46
Show file tree
Hide file tree
Showing 13 changed files with 306 additions and 7 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/extensions/api/developer_private/developer_private_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,12 @@ std::unique_ptr<developer::ProfileInfo> CreateProfileInfo(Profile* profile) {
std::unique_ptr<developer::ProfileInfo> info(new developer::ProfileInfo());
info->is_supervised = profile->IsSupervised();
PrefService* prefs = profile->GetPrefs();
const PrefService::Preference* pref =
prefs->FindPreference(prefs::kExtensionsUIDeveloperMode);
info->is_incognito_available =
IncognitoModePrefs::GetAvailability(prefs) !=
IncognitoModePrefs::DISABLED;
info->is_developer_mode_controlled_by_policy = pref->IsManaged();
info->in_developer_mode =
!info->is_supervised &&
prefs->GetBoolean(prefs::kExtensionsUIDeveloperMode);
Expand Down
93 changes: 92 additions & 1 deletion chrome/browser/extensions/api/developer_private/developer_private_api_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
#include "chrome/common/pref_names.h"
#include "chrome/test/base/test_browser_window.h"
#include "components/crx_file/id_util.h"
#include "components/policy/core/browser/browser_policy_connector.h"
#include "components/policy/core/common/mock_configuration_policy_provider.h"
#include "components/policy/core/common/policy_map.h"
#include "components/policy/core/common/policy_service_impl.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/policy_constants.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/test/web_contents_tester.h"
#include "extensions/browser/event_router_factory.h"
#include "extensions/browser/extension_error_test_util.h"
Expand All @@ -40,6 +47,9 @@
#include "extensions/common/test_util.h"
#include "extensions/common/value_builder.h"

using testing::Return;
using testing::_;

namespace extensions {

namespace {
Expand Down Expand Up @@ -95,6 +105,18 @@ class DeveloperPrivateApiUnitTest : public ExtensionServiceTestBase {
api::developer_private::PackStatus expected_status,
int expected_flags);

// Execute the updateProfileConfiguration API call with a specified
// dev_mode. This is done from the webui when the user checks the
// "Developer Mode" checkbox.
void UpdateProfileConfigurationDevMode(bool dev_mode);

// Execute the getProfileConfiguration API and parse its result into a
// ProfileInfo structure for further verification in the calling test.
// Will reset the profile_info unique_ptr.
// Uses ASSERT_* inside - callers should use ASSERT_NO_FATAL_FAILURE.
void GetProfileConfiguration(
std::unique_ptr<api::developer_private::ProfileInfo>* profile_info);

Browser* browser() { return browser_.get(); }

private:
Expand All @@ -107,6 +129,7 @@ class DeveloperPrivateApiUnitTest : public ExtensionServiceTestBase {
std::unique_ptr<Browser> browser_;

std::vector<std::unique_ptr<TestExtensionDir>> test_extension_dirs_;
policy::MockConfigurationPolicyProvider mock_policy_provider_;

DISALLOW_COPY_AND_ASSIGN(DeveloperPrivateApiUnitTest);
};
Expand Down Expand Up @@ -242,9 +265,43 @@ testing::AssertionResult DeveloperPrivateApiUnitTest::TestPackExtensionFunction(
return testing::AssertionSuccess();
}

void DeveloperPrivateApiUnitTest::UpdateProfileConfigurationDevMode(
bool dev_mode) {
scoped_refptr<UIThreadExtensionFunction> function(
new api::DeveloperPrivateUpdateProfileConfigurationFunction());
std::unique_ptr<base::ListValue> args =
ListBuilder()
.Append(DictionaryBuilder()
.SetBoolean("inDeveloperMode", dev_mode)
.Build())
.Build();
EXPECT_TRUE(RunFunction(function, *args)) << function->GetError();
}

void DeveloperPrivateApiUnitTest::GetProfileConfiguration(
std::unique_ptr<api::developer_private::ProfileInfo>* profile_info) {
scoped_refptr<UIThreadExtensionFunction> function(
new api::DeveloperPrivateGetProfileConfigurationFunction());
base::ListValue args;
EXPECT_TRUE(RunFunction(function, args)) << function->GetError();

ASSERT_TRUE(function->GetResultList());
ASSERT_EQ(1u, function->GetResultList()->GetSize());
const base::Value* response_value = nullptr;
function->GetResultList()->Get(0u, &response_value);
*profile_info =
api::developer_private::ProfileInfo::FromValue(*response_value);
}

void DeveloperPrivateApiUnitTest::SetUp() {
ExtensionServiceTestBase::SetUp();
InitializeEmptyExtensionService();

// By not specifying a pref_file filepath, we get a
// sync_preferences::TestingPrefServiceSyncable
// - see BuildTestingProfile in extension_service_test_base.cc.
ExtensionServiceInitParams init_params = CreateDefaultInitParams();
init_params.pref_file.clear();
InitializeExtensionService(init_params);

browser_window_.reset(new TestBrowserWindow());
Browser::CreateParams params(profile());
Expand Down Expand Up @@ -576,4 +633,38 @@ TEST_F(DeveloperPrivateApiUnitTest, DeveloperPrivateDeleteExtensionErrors) {
EXPECT_TRUE(error_console->GetErrorsForExtension(extension->id()).empty());
}

// Test developerPrivate.updateProfileConfiguration: Try to turn on devMode
// when DeveloperToolsDisabled policy is active.
TEST_F(DeveloperPrivateApiUnitTest, DeveloperPrivateDevModeDisabledPolicy) {
testing_pref_service()->SetManagedPref(prefs::kExtensionsUIDeveloperMode,
new base::FundamentalValue(false));

UpdateProfileConfigurationDevMode(true);

EXPECT_FALSE(
profile()->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode));

std::unique_ptr<api::developer_private::ProfileInfo> profile_info;
ASSERT_NO_FATAL_FAILURE(GetProfileConfiguration(&profile_info));
EXPECT_FALSE(profile_info->in_developer_mode);
EXPECT_TRUE(profile_info->is_developer_mode_controlled_by_policy);
}

// Test developerPrivate.updateProfileConfiguration: Try to turn on devMode
// (without DeveloperToolsDisabled policy).
TEST_F(DeveloperPrivateApiUnitTest, DeveloperPrivateDevMode) {
EXPECT_FALSE(
profile()->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode));

UpdateProfileConfigurationDevMode(true);

EXPECT_TRUE(
profile()->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode));

std::unique_ptr<api::developer_private::ProfileInfo> profile_info;
ASSERT_NO_FATAL_FAILURE(GetProfileConfiguration(&profile_info));
EXPECT_TRUE(profile_info->in_developer_mode);
EXPECT_FALSE(profile_info->is_developer_mode_controlled_by_policy);
}

} // namespace extensions
5 changes: 5 additions & 0 deletions chrome/browser/extensions/extension_service_test_base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,11 @@ Profile* ExtensionServiceTestBase::profile() {
return profile_.get();
}

sync_preferences::TestingPrefServiceSyncable*
ExtensionServiceTestBase::testing_pref_service() {
return profile_->GetTestingPrefService();
}

void ExtensionServiceTestBase::CreateExtensionService(
const ExtensionServiceInitParams& params) {
TestExtensionSystem* system =
Expand Down
11 changes: 8 additions & 3 deletions chrome/browser/extensions/extension_service_test_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ namespace content {
class BrowserContext;
}

namespace sync_preferences {
class TestingPrefServiceSyncable;
}

namespace extensions {

class ExtensionRegistry;
Expand All @@ -51,9 +55,9 @@ class ExtensionServiceTestBase : public testing::Test {
base::FilePath profile_path;
base::FilePath pref_file;
base::FilePath extensions_install_dir;
bool autoupdate_enabled; // defaults to false.
bool is_first_run; // defaults to true.
bool profile_is_supervised; // defaults to false.
bool autoupdate_enabled; // defaults to false.
bool is_first_run; // defaults to true.
bool profile_is_supervised; // defaults to false.

// Though you could use this constructor, you probably want to use
// CreateDefaultInitParams(), and then make a change or two.
Expand Down Expand Up @@ -117,6 +121,7 @@ class ExtensionServiceTestBase : public testing::Test {

content::BrowserContext* browser_context();
Profile* profile();
sync_preferences::TestingPrefServiceSyncable* testing_pref_service();
ExtensionService* service() { return service_; }
ExtensionRegistry* registry() { return registry_; }
const base::FilePath& extensions_install_dir() const {
Expand Down
26 changes: 26 additions & 0 deletions chrome/browser/policy/configuration_policy_handler_list_factory.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,31 @@ void GetExtensionAllowedTypesMap(
new base::FundamentalValue(entry.manifest_type))));
}
}

// Piggy-back kDeveloperToolsDisabled set to true to also force-disable
// kExtensionsUIDeveloperMode.
class DevToolsExtensionsUIPolicyHandler : public TypeCheckingPolicyHandler {
public:
DevToolsExtensionsUIPolicyHandler()
: TypeCheckingPolicyHandler(key::kDeveloperToolsDisabled,
base::Value::Type::BOOLEAN) {}
~DevToolsExtensionsUIPolicyHandler() override {}

// ConfigurationPolicyHandler implementation:
void ApplyPolicySettings(const PolicyMap& policies,
PrefValueMap* prefs) override {
const base::Value* value = policies.GetValue(policy_name());
bool developerToolsDisabled;
if (value && value->GetAsBoolean(&developerToolsDisabled) &&
developerToolsDisabled) {
prefs->SetValue(prefs::kExtensionsUIDeveloperMode,
base::MakeUnique<base::FundamentalValue>(false));
}
}

private:
DISALLOW_COPY_AND_ASSIGN(DevToolsExtensionsUIPolicyHandler);
};
#endif

void GetDeprecatedFeaturesMap(
Expand Down Expand Up @@ -798,6 +823,7 @@ std::unique_ptr<ConfigurationPolicyHandlerList> BuildHandlerList(
handlers->AddHandler(
base::MakeUnique<extensions::ExtensionSettingsPolicyHandler>(
chrome_schema));
handlers->AddHandler(base::MakeUnique<DevToolsExtensionsUIPolicyHandler>());
#endif

#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
Expand Down
92 changes: 92 additions & 0 deletions chrome/browser/policy/policy_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,98 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, DeveloperToolsDisabled) {
EXPECT_FALSE(DevToolsWindow::GetInstanceForInspectedWebContents(contents));
}

namespace {

// Utility for waiting until the dev-mode controls are visible/hidden
// Uses a MutationObserver on the attributes of the DOM element.
void WaitForExtensionsDevModeControlsVisibility(
content::WebContents* contents,
const char* dev_controls_accessor_js,
const char* dev_controls_visibility_check_js,
bool expected_visible) {
bool done = false;
ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
contents,
base::StringPrintf(
"var screenElement = %s;"
"function SendReplyIfAsExpected() {"
" var is_visible = %s;"
" if (is_visible != %s)"
" return false;"
" observer.disconnect();"
" domAutomationController.send(true);"
" return true;"
"}"
"var observer = new MutationObserver(SendReplyIfAsExpected);"
"if (!SendReplyIfAsExpected()) {"
" var options = { 'attributes': true };"
" observer.observe(screenElement, options);"
"}",
dev_controls_accessor_js,
dev_controls_visibility_check_js,
(expected_visible ? "true" : "false")),
&done));
}

} // namespace

IN_PROC_BROWSER_TEST_F(PolicyTest, DeveloperToolsDisabledExtensionsDevMode) {
// Verifies that when DeveloperToolsDisabled policy is set, the "dev mode"
// in chrome://extensions-frame is actively turned off and the checkbox
// is disabled.
// Note: We don't test the indicator as it is tested in the policy pref test
// for kDeveloperToolsDisabled.

// This test currently depends on the following assumptions about the webui:
// (1) The ID of the checkbox to toggle dev mode
const char toggle_dev_mode_accessor_js[] =
"document.getElementById('toggle-dev-on')";
// (2) The ID of the dev controls containing element
const char dev_controls_accessor_js[] =
"document.getElementById('dev-controls')";
// (3) the fact that dev-controls is displayed/hidden using its height attr
const char dev_controls_visibility_check_js[] =
"parseFloat(document.getElementById('dev-controls').style.height) > 0";

// Navigate to the extensions frame and enabled "Developer mode"
ui_test_utils::NavigateToURL(browser(),
GURL(chrome::kChromeUIExtensionsFrameURL));

content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
EXPECT_TRUE(content::ExecuteScript(
contents, base::StringPrintf("domAutomationController.send(%s.click());",
toggle_dev_mode_accessor_js)));

WaitForExtensionsDevModeControlsVisibility(contents,
dev_controls_accessor_js,
dev_controls_visibility_check_js,
true);

// Disable devtools via policy.
PolicyMap policies;
policies.Set(key::kDeveloperToolsDisabled, POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
base::MakeUnique<base::FundamentalValue>(true), nullptr);
UpdateProviderPolicy(policies);

// Expect devcontrols to be hidden now...
WaitForExtensionsDevModeControlsVisibility(contents,
dev_controls_accessor_js,
dev_controls_visibility_check_js,
false);

// ... and checkbox is not disabled
bool is_toggle_dev_mode_checkbox_enabled = false;
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
contents,
base::StringPrintf(
"domAutomationController.send(!%s.hasAttribute('disabled'))",
toggle_dev_mode_accessor_js),
&is_toggle_dev_mode_checkbox_enabled));
EXPECT_FALSE(is_toggle_dev_mode_checkbox_enabled);
}

// TODO(samarth): remove along with rest of NTP4 code.
IN_PROC_BROWSER_TEST_F(PolicyTest, DISABLED_WebStoreIconHidden) {
// Verifies that the web store icons can be hidden from the new tab page.
Expand Down
Loading

0 comments on commit 6e36b46

Please sign in to comment.