Skip to content

GitLab

T
tangled
Commit 3e46ef2d authored by kustermann@google.com's avatar kustermann@google.com
Browse files
Options

Revert 282925 "Make the policy fetch for first time login blocking" 

> Make the policy fetch for first time login blocking
>
> The CL makes policy fetching for first time login blocking for all users, except the ones that are known to be non-enterprise users.
>
> BUG=334584
>
> Review URL: https://codereview.chromium.org/330843002


Reason for revert:
The "Linux Chromium OS ASan LSan Tests (3)" tester found memory leaks.

TBR=kaliamoorthi@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282942 0039d316-1c4b-4281-b951-d872f2087c98
parent 51ed7bd4
Hide whitespace changes
Inline Side-by-side
Showing with 63 additions and 255 deletions
chrome/browser/chromeos/login/login_utils_browsertest.cc
View file @ 3e46ef2d
......@@ -528,15 +528,49 @@ class LoginUtilsTest : public testing::Test,
DISALLOW_COPY_AND_ASSIGN(LoginUtilsTest);
};
struct LoginUtilsBlockingLoginTestParam {
const int steps;
const char* username;
const bool enroll_device;
};
class LoginUtilsBlockingLoginTest
: public LoginUtilsTest,
public testing::WithParamInterface<LoginUtilsBlockingLoginTestParam> {};
public testing::WithParamInterface<int> {};
TEST_F(LoginUtilsTest, NormalLoginDoesntBlock) {
UserManager* user_manager = UserManager::Get();
EXPECT_FALSE(user_manager->IsUserLoggedIn());
EXPECT_FALSE(connector_->IsEnterpriseManaged());
EXPECT_FALSE(prepared_profile_);
EXPECT_EQ(policy::USER_AFFILIATION_NONE,
connector_->GetUserAffiliation(kUsername));
// The profile will be created without waiting for a policy response.
PrepareProfile(kUsername);
EXPECT_TRUE(prepared_profile_);
ASSERT_TRUE(user_manager->IsUserLoggedIn());
EXPECT_EQ(kUsername, user_manager->GetLoggedInUser()->email());
}
TEST_F(LoginUtilsTest, EnterpriseLoginDoesntBlockForNormalUser) {
UserManager* user_manager = UserManager::Get();
EXPECT_FALSE(user_manager->IsUserLoggedIn());
EXPECT_FALSE(connector_->IsEnterpriseManaged());
EXPECT_FALSE(prepared_profile_);
// Enroll the device.
EnrollDevice(kUsername);
EXPECT_FALSE(user_manager->IsUserLoggedIn());
EXPECT_TRUE(connector_->IsEnterpriseManaged());
EXPECT_EQ(kDomain, connector_->GetEnterpriseDomain());
EXPECT_FALSE(prepared_profile_);
EXPECT_EQ(policy::USER_AFFILIATION_NONE,
connector_->GetUserAffiliation(kUsernameOtherDomain));
// Login with a non-enterprise user shouldn't block.
PrepareProfile(kUsernameOtherDomain);
EXPECT_TRUE(prepared_profile_);
ASSERT_TRUE(user_manager->IsUserLoggedIn());
EXPECT_EQ(kUsernameOtherDomain, user_manager->GetLoggedInUser()->email());
}
#if defined(ENABLE_RLZ)
TEST_F(LoginUtilsTest, RlzInitialized) {
......@@ -564,26 +598,25 @@ TEST_F(LoginUtilsTest, RlzInitialized) {
}
#endif
TEST_P(LoginUtilsBlockingLoginTest, LoginBlocksForUser) {
TEST_P(LoginUtilsBlockingLoginTest, EnterpriseLoginBlocksForEnterpriseUser) {
UserManager* user_manager = UserManager::Get();
EXPECT_FALSE(user_manager->IsUserLoggedIn());
EXPECT_FALSE(connector_->IsEnterpriseManaged());
EXPECT_FALSE(prepared_profile_);
if (GetParam().enroll_device) {
// Enroll the device.
EnrollDevice(kUsername);
// Enroll the device.
EnrollDevice(kUsername);
EXPECT_FALSE(user_manager->IsUserLoggedIn());
EXPECT_TRUE(connector_->IsEnterpriseManaged());
EXPECT_EQ(kDomain, connector_->GetEnterpriseDomain());
EXPECT_FALSE(prepared_profile_);
EXPECT_EQ(policy::USER_AFFILIATION_MANAGED,
connector_->GetUserAffiliation(kUsername));
EXPECT_FALSE(user_manager->IsKnownUser(kUsername));
}
EXPECT_FALSE(user_manager->IsUserLoggedIn());
EXPECT_TRUE(connector_->IsEnterpriseManaged());
EXPECT_EQ(kDomain, connector_->GetEnterpriseDomain());
EXPECT_FALSE(prepared_profile_);
EXPECT_EQ(policy::USER_AFFILIATION_MANAGED,
connector_->GetUserAffiliation(kUsername));
EXPECT_FALSE(user_manager->IsKnownUser(kUsername));
PrepareProfile(GetParam().username);
// Login with a user of the enterprise domain waits for policy.
PrepareProfile(kUsername);
EXPECT_FALSE(prepared_profile_);
ASSERT_TRUE(user_manager->IsUserLoggedIn());
......@@ -595,7 +628,7 @@ TEST_P(LoginUtilsBlockingLoginTest, LoginBlocksForUser) {
// |steps| is the test parameter, and is the number of successful fetches.
// The first incomplete fetch will fail. In any case, the profile creation
// should resume.
int steps = GetParam().steps;
int steps = GetParam();
// The next expected fetcher ID. This is used to make it fail.
int next_expected_fetcher_id = 0;
......@@ -670,29 +703,10 @@ TEST_P(LoginUtilsBlockingLoginTest, LoginBlocksForUser) {
EXPECT_TRUE(prepared_profile_);
}
const LoginUtilsBlockingLoginTestParam kBlockinLoginTestCases[] = {
{0, kUsername, true},
{1, kUsername, true},
{2, kUsername, true},
{3, kUsername, true},
{4, kUsername, true},
{5, kUsername, true},
{0, kUsername, false},
{1, kUsername, false},
{2, kUsername, false},
{3, kUsername, false},
{4, kUsername, false},
{5, kUsername, false},
{0, kUsernameOtherDomain, true},
{1, kUsernameOtherDomain, true},
{2, kUsernameOtherDomain, true},
{3, kUsernameOtherDomain, true},
{4, kUsernameOtherDomain, true},
{5, kUsernameOtherDomain, true}};
INSTANTIATE_TEST_CASE_P(LoginUtilsBlockingLoginTestInstance,
LoginUtilsBlockingLoginTest,
testing::ValuesIn(kBlockinLoginTestCases));
INSTANTIATE_TEST_CASE_P(
LoginUtilsBlockingLoginTestInstance,
LoginUtilsBlockingLoginTest,
testing::Values(0, 1, 2, 3, 4, 5));
} // namespace
......
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
View file @ 3e46ef2d
......@@ -91,7 +91,7 @@ UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS(
wait_for_policy_fetch_(wait_for_policy_fetch),
policy_fetch_timeout_(false, false) {
time_init_started_ = base::Time::Now();
if (wait_for_policy_fetch_ && !initial_policy_fetch_timeout.is_max()) {
if (wait_for_policy_fetch_) {
policy_fetch_timeout_.Start(
FROM_HERE,
initial_policy_fetch_timeout,
......
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_browsertest.cc deleted 100644 → 0
View file @ 51ed7bd4
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <string>
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/file_util.h"
#include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.h"
#include "base/macros.h"
#include "base/memory/scoped_ptr.h"
#include "base/strings/stringprintf.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/login/test/oobe_base_test.h"
#include "chrome/browser/chromeos/login/wizard_controller.h"
#include "chrome/browser/policy/test/local_policy_test_server.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/host_desktop.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
#include "components/policy/core/common/cloud/cloud_policy_constants.h"
#include "components/policy/core/common/policy_switches.h"
#include "content/public/browser/notification_service.h"
#include "content/public/test/test_utils.h"
#include "google_apis/gaia/fake_gaia.h"
#include "google_apis/gaia/gaia_constants.h"
#include "google_apis/gaia/gaia_urls.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace policy {
namespace {
const char kAccountId[] = "dla1@example.com";
const char kAccountPassword[] = "letmein";
const char* kStartupURLs[] = {"chrome://policy", "chrome://about"};
const char kTestAuthCode[] = "fake-auth-code";
const char kTestGaiaUberToken[] = "fake-uber-token";
const char kTestAuthLoginAccessToken[] = "fake-access-token";
const char kTestRefreshToken[] = "fake-refresh-token";
const char kTestAuthSIDCookie[] = "fake-auth-SID-cookie";
const char kTestAuthLSIDCookie[] = "fake-auth-LSID-cookie";
const char kTestSessionSIDCookie[] = "fake-session-SID-cookie";
const char kTestSessionLSIDCookie[] = "fake-session-LSID-cookie";
const char kTestUserinfoToken[] = "fake-userinfo-token";
} // namespace
class UserCloudPolicyManagerTest : public chromeos::OobeBaseTest {
protected:
UserCloudPolicyManagerTest() {
set_open_about_blank_on_browser_launch(false);
}
virtual ~UserCloudPolicyManagerTest() {}
virtual void SetUp() OVERRIDE {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
SetServerPolicy();
test_server_.reset(new LocalPolicyTestServer(policy_file_path()));
ASSERT_TRUE(test_server_->Start());
OobeBaseTest::SetUp();
}
virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
command_line->AppendSwitchASCII(policy::switches::kDeviceManagementUrl,
test_server_->GetServiceURL().spec());
OobeBaseTest::SetUpCommandLine(command_line);
}
virtual void SetUpOnMainThread() OVERRIDE {
SetMergeSessionParams(kAccountId);
SetupGaiaServerWithAccessTokens();
OobeBaseTest::SetUpOnMainThread();
}
void SetupGaiaServerWithAccessTokens() {
FakeGaia::AccessTokenInfo token_info;
token_info.token = kTestUserinfoToken;
token_info.scopes.insert(GaiaConstants::kDeviceManagementServiceOAuth);
token_info.scopes.insert(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
token_info.audience = GaiaUrls::GetInstance()->oauth2_chrome_client_id();
token_info.email = kAccountId;
fake_gaia_->IssueOAuthToken(kTestRefreshToken, token_info);
}
void SetMergeSessionParams(const std::string& email) {
FakeGaia::MergeSessionParams params;
params.auth_sid_cookie = kTestAuthSIDCookie;
params.auth_lsid_cookie = kTestAuthLSIDCookie;
params.auth_code = kTestAuthCode;
params.refresh_token = kTestRefreshToken;
params.access_token = kTestAuthLoginAccessToken;
params.gaia_uber_token = kTestGaiaUberToken;
params.session_sid_cookie = kTestSessionSIDCookie;
params.session_lsid_cookie = kTestSessionLSIDCookie;
params.email = email;
fake_gaia_->SetMergeSessionParams(params);
}
void SkipToLoginScreen() {
chromeos::WizardController::SkipPostLoginScreensForTesting();
chromeos::WizardController* wizard_controller =
chromeos::WizardController::default_controller();
ASSERT_TRUE(wizard_controller);
wizard_controller->SkipToLoginForTesting(chromeos::LoginScreenContext());
content::WindowedNotificationObserver(
chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
content::NotificationService::AllSources()).Wait();
}
void LogIn(const std::string& user_id, const std::string& password) {
GetLoginDisplay()->ShowSigninScreenForCreds(user_id, password);
content::WindowedNotificationObserver(
chrome::NOTIFICATION_SESSION_STARTED,
content::NotificationService::AllSources()).Wait();
}
void SetServerPolicy() {
const char kPolicy[] =
"{"
" \"%s\": {"
" \"mandatory\": {"
" \"RestoreOnStartup\": 4,"
" \"RestoreOnStartupURLs\": ["
" \"chrome://policy\","
" \"chrome://about\""
" ]"
" },"
" \"recommended\": {}"
" },"
" \"managed_users\": [ \"*\" ],"
" \"policy_user\": \"%s\","
" \"current_key_index\": 0"
"}";
const std::string policy = base::StringPrintf(
kPolicy, dm_protocol::kChromeUserPolicyType, kAccountId);
const int bytes_written =
base::WriteFile(policy_file_path(), policy.data(), policy.size());
ASSERT_EQ(static_cast<int>(policy.size()), bytes_written);
}
base::FilePath policy_file_path() const {
return temp_dir_.path().AppendASCII("policy.json");
}
scoped_ptr<LocalPolicyTestServer> test_server_;
base::ScopedTempDir temp_dir_;
private:
DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyManagerTest);
};
IN_PROC_BROWSER_TEST_F(UserCloudPolicyManagerTest, StartSession) {
SkipToLoginScreen();
LogIn(kAccountId, kAccountPassword);
// Check that the startup pages specified in policy were opened.
BrowserList* browser_list =
BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
EXPECT_EQ(1U, browser_list->size());
Browser* browser = browser_list->get(0);
ASSERT_TRUE(browser);
TabStripModel* tabs = browser->tab_strip_model();
ASSERT_TRUE(tabs);
const int expected_tab_count = static_cast<int>(arraysize(kStartupURLs));
EXPECT_EQ(expected_tab_count, tabs->count());
for (int i = 0; i < expected_tab_count && i < tabs->count(); ++i) {
EXPECT_EQ(GURL(kStartupURLs[i]),
tabs->GetWebContentsAt(i)->GetVisibleURL());
}
}
} // namespace policy
chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc
View file @ 3e46ef2d
......@@ -17,7 +17,6 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/login/login_utils.h"
#include "chrome/browser/chromeos/login/users/user.h"
#include "chrome/browser/chromeos/login/users/user_manager.h"
#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
#include "chrome/browser/chromeos/policy/user_cloud_external_data_manager.h"
#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
......@@ -30,7 +29,6 @@
#include "chromeos/chromeos_switches.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/policy/core/browser/browser_policy_connector.h"
#include "components/policy/core/common/cloud/cloud_external_data_manager.h"
#include "components/policy/core/common/cloud/device_management_service.h"
#include "components/user_manager/user_type.h"
......@@ -135,7 +133,6 @@ scoped_ptr<UserCloudPolicyManagerChromeOS>
// policy registration.
// USER_TYPE_PUBLIC_ACCOUNT gets its policy from the
// DeviceLocalAccountPolicyService.
// Non-managed domains will be skipped by the below check
const std::string& username = user->email();
if (user->GetType() != user_manager::USER_TYPE_REGULAR ||
BrowserPolicyConnector::IsNonEnterpriseUser(username)) {
......@@ -145,17 +142,10 @@ scoped_ptr<UserCloudPolicyManagerChromeOS>
policy::BrowserPolicyConnectorChromeOS* connector =
g_browser_process->platform_part()->browser_policy_connector_chromeos();
UserAffiliation affiliation = connector->GetUserAffiliation(username);
const bool is_affiliated_user = affiliation == USER_AFFILIATION_MANAGED;
const bool is_managed_user = affiliation == USER_AFFILIATION_MANAGED;
const bool is_browser_restart =
command_line->HasSwitch(chromeos::switches::kLoginUser);
const bool wait_for_initial_policy =
!is_browser_restart &&
(chromeos::UserManager::Get()->IsCurrentUserNew() || is_affiliated_user);
const base::TimeDelta initial_policy_fetch_timeout =
chromeos::UserManager::Get()->IsCurrentUserNew()
? base::TimeDelta::Max()
: base::TimeDelta::FromSeconds(kInitialPolicyFetchTimeoutSeconds);
const bool wait_for_initial_policy = is_managed_user && !is_browser_restart;
DeviceManagementService* device_management_service =
connector->device_management_service();
......@@ -205,7 +195,7 @@ scoped_ptr<UserCloudPolicyManagerChromeOS>
external_data_manager.Pass(),
component_policy_cache_dir,
wait_for_initial_policy,
initial_policy_fetch_timeout,
base::TimeDelta::FromSeconds(kInitialPolicyFetchTimeoutSeconds),
base::MessageLoopProxy::current(),
file_task_runner,
io_task_runner));
......
chrome/chrome_tests.gypi
View file @ 3e46ef2d
......@@ -1025,7 +1025,6 @@
'browser/chromeos/policy/policy_cert_verifier_browsertest.cc',
'browser/chromeos/policy/power_policy_browsertest.cc',
'browser/chromeos/policy/user_cloud_external_data_manager_browsertest.cc',
'browser/chromeos/policy/user_cloud_policy_manager_chromeos_browsertest.cc',
'browser/chromeos/policy/variations_service_policy_browsertest.cc',
'browser/chromeos/power/peripheral_battery_observer_browsertest.cc',
'browser/chromeos/preferences_browsertest.cc',
......
chrome/test/base/in_process_browser_test.cc
View file @ 3e46ef2d
......@@ -112,7 +112,6 @@ void SingleDesktopTestObserver::OnBrowserAdded(Browser* browser) {
InProcessBrowserTest::InProcessBrowserTest()
: browser_(NULL),
exit_when_last_browser_closes_(true),
open_about_blank_on_browser_launch_(true),
multi_desktop_test_(false)
#if defined(OS_MACOSX)
, autorelease_pool_(NULL)
......@@ -249,7 +248,7 @@ void InProcessBrowserTest::PrepareTestCommandLine(CommandLine* command_line) {
if (exit_when_last_browser_closes_)
command_line->AppendSwitch(switches::kDisableZeroBrowsersOpenForTests);
if (open_about_blank_on_browser_launch_ && command_line->GetArgs().empty())
if (command_line->GetArgs().empty())
command_line->AppendArg(url::kAboutBlankURL);
}
......
chrome/test/base/in_process_browser_test.h
View file @ 3e46ef2d
......@@ -171,10 +171,6 @@ class InProcessBrowserTest : public content::BrowserTestBase {
exit_when_last_browser_closes_ = value;
}
void set_open_about_blank_on_browser_launch(bool value) {
open_about_blank_on_browser_launch_ = value;
}
// This must be called before RunTestOnMainThreadLoop() to have any effect.
void set_multi_desktop_test(bool multi_desktop_test) {
multi_desktop_test_ = multi_desktop_test;
......@@ -202,9 +198,6 @@ class InProcessBrowserTest : public content::BrowserTestBase {
// True if we should exit the tests after the last browser instance closes.
bool exit_when_last_browser_closes_;
// True if the about:blank tab should be opened when the browser is launched.
bool open_about_blank_on_browser_launch_;
// True if this is a multi-desktop test (in which case this browser test will
// not ensure that Browsers are only created on the tested desktop).
bool multi_desktop_test_;
......
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