Skip to content

Commit

Permalink
Enable Enterprise enrollment on desktop builds.
Browse files Browse the repository at this point in the history
This change implements some of the DBus stub methods so that enterprise enrollment works on desktop builds. That will make development of features that depend on enrollment faster for developers that use this workflow (e.g. for kiosk enterprise apps, public accounts, testing some device policies, etc).

- Override some of the directories and files involved with the enrollment state
- Simple stub implementation of the DBus calls involved
- Write a persistent cache of the install attributes
- Cleaned up the stub for user cloud policy and made them persistent too
- Updated some tests

This change doesn't affect production code.

TBR=jochen@chromium.org
BUG=240269, 367674

Review URL: https://codereview.chromium.org/258743005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267640 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
joaodasilva@chromium.org committed May 1, 2014
1 parent 8f471ef commit 63ebbf2
Show file tree
Hide file tree
Showing 27 changed files with 445 additions and 164 deletions.
15 changes: 10 additions & 5 deletions base/path_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,15 @@ bool PathService::Get(int key, FilePath* result) {

// static
bool PathService::Override(int key, const FilePath& path) {
// Just call the full function with true for the value of |create|.
return OverrideAndCreateIfNeeded(key, path, true);
// Just call the full function with true for the value of |create|, and
// assume that |path| may not be absolute yet.
return OverrideAndCreateIfNeeded(key, path, false, true);
}

// static
bool PathService::OverrideAndCreateIfNeeded(int key,
const FilePath& path,
bool is_absolute,
bool create) {
PathData* path_data = GetPathData();
DCHECK(path_data);
Expand All @@ -259,9 +261,12 @@ bool PathService::OverrideAndCreateIfNeeded(int key,
}

// We need to have an absolute path.
file_path = MakeAbsoluteFilePath(file_path);
if (file_path.empty())
return false;
if (!is_absolute) {
file_path = MakeAbsoluteFilePath(file_path);
if (file_path.empty())
return false;
}
DCHECK(file_path.IsAbsolute());

base::AutoLock scoped_lock(path_data->lock);

Expand Down
11 changes: 9 additions & 2 deletions base/path_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,18 @@ class BASE_EXPORT PathService {
// one test should not carry over to another.
static bool Override(int key, const base::FilePath& path);

// This function does the same as PathService::Override but it takes an extra
// parameter |create| which guides whether the directory to be overriden must
// This function does the same as PathService::Override but it takes extra
// parameters:
// - |is_absolute| indicates that |path| has already been expanded into an
// absolute path, otherwise MakeAbsoluteFilePath() will be used. This is
// useful to override paths that may not exist yet, since MakeAbsoluteFilePath
// fails for those. Note that MakeAbsoluteFilePath also expands symbolic
// links, even if path.IsAbsolute() is already true.
// - |create| guides whether the directory to be overriden must
// be created in case it doesn't exist already.
static bool OverrideAndCreateIfNeeded(int key,
const base::FilePath& path,
bool is_absolute,
bool create);

// To extend the set of supported keys, you can register a path provider,
Expand Down
27 changes: 26 additions & 1 deletion base/path_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ TEST_F(PathServiceTest, Get) {
#endif
}

// test that all versions of the Override function of PathService do what they
// Test that all versions of the Override function of PathService do what they
// are supposed to do.
TEST_F(PathServiceTest, Override) {
int my_special_key = 666;
Expand All @@ -163,12 +163,37 @@ TEST_F(PathServiceTest, Override) {
// PathService::OverrideAndCreateIfNeeded should obey the |create| parameter.
PathService::OverrideAndCreateIfNeeded(my_special_key,
fake_cache_dir2,
false,
false);
EXPECT_FALSE(base::PathExists(fake_cache_dir2));
EXPECT_TRUE(PathService::OverrideAndCreateIfNeeded(my_special_key,
fake_cache_dir2,
false,
true));
EXPECT_TRUE(base::PathExists(fake_cache_dir2));

#if defined(OS_POSIX)
base::FilePath non_existent(
base::MakeAbsoluteFilePath(temp_dir.path()).AppendASCII("non_existent"));
EXPECT_TRUE(non_existent.IsAbsolute());
EXPECT_FALSE(base::PathExists(non_existent));
// This fails because MakeAbsoluteFilePath fails for non-existent files.
EXPECT_FALSE(PathService::OverrideAndCreateIfNeeded(my_special_key,
non_existent,
false,
false));
// This works because indicating that |non_existent| is absolute skips the
// internal MakeAbsoluteFilePath call.
EXPECT_TRUE(PathService::OverrideAndCreateIfNeeded(my_special_key,
non_existent,
true,
false));
// Check that the path has been overridden and no directory was created.
EXPECT_FALSE(base::PathExists(non_existent));
base::FilePath path;
EXPECT_TRUE(PathService::Get(my_special_key, &path));
EXPECT_EQ(non_existent, path);
#endif
}

// Check if multiple overrides can co-exist.
Expand Down
2 changes: 1 addition & 1 deletion chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ void InitializeUserDataDir() {

const bool specified_directory_was_invalid = !user_data_dir.empty() &&
!PathService::OverrideAndCreateIfNeeded(chrome::DIR_USER_DATA,
user_data_dir, true);
user_data_dir, false, true);
// Save inaccessible or invalid paths so the user may be prompted later.
if (specified_directory_was_invalid)
chrome::SetInvalidSpecifiedUserDataDir(user_data_dir);
Expand Down
19 changes: 9 additions & 10 deletions chrome/browser/chromeos/chrome_browser_main_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,16 +247,6 @@ namespace internal {
class DBusServices {
public:
explicit DBusServices(const content::MainFunctionParams& parameters) {
if (!base::SysInfo::IsRunningOnChromeOS()) {
// Override this path on the desktop, so that the user policy key can be
// stored by the stub SessionManagerClient.
base::FilePath user_data_dir;
if (PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
PathService::Override(chromeos::DIR_USER_POLICY_KEYS,
user_data_dir.AppendASCII("stub_user_policy"));
}
}

// Initialize DBusThreadManager for the browser. This must be done after
// the main message loop is started, as it uses the message loop.
DBusThreadManager::Initialize();
Expand Down Expand Up @@ -404,6 +394,15 @@ void ChromeBrowserMainPartsChromeos::PreMainMessageLoopStart() {
}

void ChromeBrowserMainPartsChromeos::PostMainMessageLoopStart() {
base::FilePath user_data_dir;
if (!base::SysInfo::IsRunningOnChromeOS() &&
PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
// Override some paths with stub locations so that cloud policy and
// enterprise enrollment work on desktop builds, for ease of
// development.
chromeos::RegisterStubPathOverrides(user_data_dir);
}

dbus_services_.reset(new internal::DBusServices(parameters()));

ChromeBrowserMainPartsLinux::PostMainMessageLoopStart();
Expand Down
18 changes: 7 additions & 11 deletions chrome/browser/chromeos/login/kiosk_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -849,15 +849,6 @@ IN_PROC_BROWSER_TEST_F(KioskTest, KioskEnableAbortedWithAutoEnrollment) {
}

IN_PROC_BROWSER_TEST_F(KioskTest, KioskEnableAfter2ndSigninScreen) {
// Fake an auto enrollment is not going to be enforced.
CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kEnterpriseEnrollmentInitialModulus, "1");
CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kEnterpriseEnrollmentModulusLimit, "2");
g_browser_process->local_state()->SetBoolean(prefs::kShouldAutoEnroll, false);
g_browser_process->local_state()->SetInteger(
prefs::kAutoEnrollmentPowerLimit, -1);

chromeos::WizardController::SkipPostLoginScreensForTesting();
chromeos::WizardController* wizard_controller =
chromeos::WizardController::default_controller();
Expand Down Expand Up @@ -1123,9 +1114,13 @@ class KioskEnterpriseTest : public KioskTest {
device_policy_test_helper_.device_policy()->policy_data();
policy_data.set_service_account_identity(kTestEnterpriseServiceAccountId);
device_policy_test_helper_.device_policy()->Build();

base::RunLoop run_loop;
DBusThreadManager::Get()->GetSessionManagerClient()->StoreDevicePolicy(
device_policy_test_helper_.device_policy()->GetBlob(),
base::Bind(&KioskEnterpriseTest::StorePolicyCallback));
base::Bind(&KioskEnterpriseTest::StorePolicyCallback,
run_loop.QuitClosure()));
run_loop.Run();

DeviceSettingsService::Get()->Load();

Expand Down Expand Up @@ -1164,8 +1159,9 @@ class KioskEnterpriseTest : public KioskTest {
base::RunLoop().RunUntilIdle();
}

static void StorePolicyCallback(bool result) {
static void StorePolicyCallback(const base::Closure& callback, bool result) {
ASSERT_TRUE(result);
callback.Run();
}

policy::DevicePolicyCrosTestHelper device_policy_test_helper_;
Expand Down
29 changes: 20 additions & 9 deletions chrome/browser/chromeos/policy/device_policy_cros_browser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "chrome/browser/chromeos/policy/device_policy_builder.h"
#include "chrome/browser/chromeos/policy/enterprise_install_attributes.h"
#include "chrome/browser/chromeos/policy/proto/install_attributes.pb.h"
#include "chrome/common/chrome_paths.h"
#include "chromeos/chromeos_paths.h"
#include "chromeos/dbus/fake_dbus_thread_manager.h"
#include "chromeos/dbus/fake_session_manager_client.h"
Expand All @@ -27,13 +28,13 @@ using ::testing::Return;

namespace policy {

DevicePolicyCrosTestHelper::DevicePolicyCrosTestHelper() {
CHECK(temp_dir_.CreateUniqueTempDir());
}
DevicePolicyCrosTestHelper::DevicePolicyCrosTestHelper() {}

DevicePolicyCrosTestHelper::~DevicePolicyCrosTestHelper() {}

void DevicePolicyCrosTestHelper::MarkAsEnterpriseOwned() {
OverridePaths();

cryptohome::SerializedInstallAttributes install_attrs_proto;
cryptohome::SerializedInstallAttributes::Attribute* attribute = NULL;

Expand All @@ -45,20 +46,22 @@ void DevicePolicyCrosTestHelper::MarkAsEnterpriseOwned() {
attribute->set_name(EnterpriseInstallAttributes::kAttrEnterpriseUser);
attribute->set_value(device_policy_.policy_data().username());

base::FilePath install_attrs_file =
temp_dir_.path().AppendASCII("install_attributes.pb");
base::FilePath install_attrs_file;
ASSERT_TRUE(
PathService::Get(chromeos::FILE_INSTALL_ATTRIBUTES, &install_attrs_file));
const std::string install_attrs_blob(
install_attrs_proto.SerializeAsString());
ASSERT_EQ(static_cast<int>(install_attrs_blob.size()),
base::WriteFile(install_attrs_file,
install_attrs_blob.c_str(),
install_attrs_blob.size()));
ASSERT_TRUE(PathService::Override(chromeos::FILE_INSTALL_ATTRIBUTES,
install_attrs_file));
}

void DevicePolicyCrosTestHelper::InstallOwnerKey() {
base::FilePath owner_key_file = temp_dir_.path().AppendASCII("owner.key");
OverridePaths();

base::FilePath owner_key_file;
ASSERT_TRUE(PathService::Get(chromeos::FILE_OWNER_KEY, &owner_key_file));
std::vector<uint8> owner_key_bits;
ASSERT_TRUE(
device_policy()->GetSigningKey()->ExportPublicKey(&owner_key_bits));
Expand All @@ -67,7 +70,15 @@ void DevicePolicyCrosTestHelper::InstallOwnerKey() {
reinterpret_cast<const char*>(vector_as_array(&owner_key_bits)),
owner_key_bits.size()),
static_cast<int>(owner_key_bits.size()));
ASSERT_TRUE(PathService::Override(chromeos::FILE_OWNER_KEY, owner_key_file));
}

void DevicePolicyCrosTestHelper::OverridePaths() {
// This is usually done by ChromeBrowserMainChromeOS, but some tests
// use the overridden paths before ChromeBrowserMain starts. Make sure that
// the paths are overridden before using them.
base::FilePath user_data_dir;
ASSERT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
chromeos::RegisterStubPathOverrides(user_data_dir);
}

DevicePolicyCrosBrowserTest::DevicePolicyCrosBrowserTest()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "base/files/scoped_temp_dir.h"
#include "chrome/browser/chromeos/policy/device_policy_builder.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chromeos/dbus/fake_dbus_thread_manager.h"
Expand All @@ -34,8 +33,7 @@ class DevicePolicyCrosTestHelper {
DevicePolicyBuilder* device_policy() { return &device_policy_; }

private:
// Stores the device owner key and the install attributes.
base::ScopedTempDir temp_dir_;
void OverridePaths();

// Carries Chrome OS device policies for tests.
DevicePolicyBuilder device_policy_;
Expand Down
Loading

0 comments on commit 63ebbf2

Please sign in to comment.