Skip to content

Commit

Permalink
Revert 250114 "Ensure that SAML IdPs use https on Chrome OS"
Browse files Browse the repository at this point in the history
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%282%29/builds/21471

[2608:2608:0210/090242:993623055:INFO:signin_screen_handler.cc(1448)] Login WebUI >> active: 0, source: gaia-signin
BrowserTestBase signal handler received SIGTERM. Backtrace:
 [0x7f74d018658e] base::debug::StackTrace::StackTrace()
 [0x0000029b7c3b] content::(anonymous namespace)::DumpStackTraceSignalHandler()
 [0x7f74c7a9c4a0] <unknown>
 [0x7f74c7b4ea43] __poll
 [0x7f74c77b7ff6] <unknown>
 [0x7f74c77b8124] g_main_context_iteration
 [0x7f74d016b0ff] base::MessagePumpGlib::RunWithDispatcher()
 [0x7f74d01c6ef0] base::MessageLoop::RunHandler()
 [0x7f74d01f2678] base::RunLoop::Run()
 [0x0000029f45cc] content::RunThisRunLoop()
 [0x0000029f4dfe] content::WindowedNotificationObserver::Wait()
 [0x000000722301] chromeos::OAuth2Test_PRE_PRE_PRE_MergeSession_Test::RunTestOnMainThread()
 [0x0000019c9a2a] InProcessBrowserTest::RunTestOnMainThreadLoop()
 [0x000002b60fb2] ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
 [0x000002b61f33] ChromeBrowserMainParts::PreMainMessageLoopRun()
 [0x0000025b4414] chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
 [0x7f74cae68484] content::BrowserMainLoop::PreMainMessageLoopRun()
 [0x7f74cb16c540] content::StartupTaskRunner::RunAllTasksNow()
 [0x7f74cae6e3c6] content::BrowserMainLoop::CreateStartupTasks()
 [0x7f74cae70930] content::BrowserMainRunnerImpl::Initialize()
 [0x7f74cae67434] content::BrowserMain()
 [0x0000029b82f1] content::BrowserTestBase::SetUp()
 [0x0000019c8eac] InProcessBrowserTest::SetUp()
 [0x0000007244f7] chromeos::OobeBaseTest::SetUp()
 [0x000001a9316a] testing::Test::Run()
 [0x000001a932aa] testing::TestInfo::Run()
 [0x000001a933ef] testing::TestCase::Run()
 [0x000001a9367f] testing::internal::UnitTestImpl::RunAllTests()
 [0x000001a8d603] testing::UnitTest::Run()
 [0x0000019e38ae] base::TestSuite::Run()
 [0x0000019c76f5] (anonymous namespace)::ChromeTestLauncherDelegate::RunTestSuite()
 [0x0000029ef267] content::LaunchTests()
 [0x0000019c7798] LaunchChromeTests()
 [0x7f74c7a8776d] __libc_start_main
 [0x00000053eca9] <unknown>

> Ensure that SAML IdPs use https on Chrome OS
> 
> This CL sets a CSP for the GAIA auth extension that ensures the entire
> login flow, including redirects to SAML IdPs, uses https.
> 
> The FakeGaia and FakeSamlIdp classes used in SAML tests use the embedded
> test server, which speaks http only. The CL therefore adds a test server
> written in Python that speaks https and acts as a wrapper by forwarding
> calls to the embedded test server over http.
> 
> TBR=asargent@chromium.org (for gaia_auth_extension_loader.cc)
> 
> BUG=337437
> TEST=Full browser test coverage
> 
> Review URL: https://codereview.chromium.org/150483002

TBR=bartfab@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@250134 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
skobes@google.com committed Feb 10, 2014
1 parent 2fdc007 commit 563e622
Show file tree
Hide file tree
Showing 13 changed files with 31 additions and 392 deletions.
9 changes: 0 additions & 9 deletions chrome/browser/chromeos/login/oobe_base_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,6 @@ void OobeBaseTest::SetUpCommandLine(CommandLine* command_line) {
command_line->AppendSwitchASCII(::switches::kGoogleApisUrl,
gaia_url.spec());
fake_gaia_->Initialize();

// Allow GAIA to be served over http.
base::FilePath test_data_dir;
ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir));
command_line->AppendSwitchASCII(
chromeos::switches::kGAIAAuthExtensionManifest,
test_data_dir.Append("login")
.Append("gaia_auth_http_manifest.json")
.value());
}

void OobeBaseTest::SimulateNetworkOffline() {
Expand Down
62 changes: 22 additions & 40 deletions chrome/browser/chromeos/login/saml/saml_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/login/existing_user_controller.h"
#include "chrome/browser/chromeos/login/login_display_host_impl.h"
#include "chrome/browser/chromeos/login/test/https_forwarder.h"
#include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
#include "chrome/browser/chromeos/login/user.h"
#include "chrome/browser/chromeos/login/user_manager.h"
Expand Down Expand Up @@ -43,7 +42,6 @@
#include "policy/policy_constants.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

using net::test_server::BasicHttpResponse;
using net::test_server::HttpRequest;
Expand All @@ -66,8 +64,7 @@ const char kTestSessionLSIDCookie[] = "fake-session-LSID-cookie";

const char kFirstSAMLUserEmail[] = "bob@example.com";
const char kSecondSAMLUserEmail[] = "alice@example.com";
const char kHTTPSAMLUserEmail[] = "carol@example.com";
const char kNonSAMLUserEmail[] = "dan@example.com";
const char kNonSAMLUserEmail[] = "carol@example.com";

const char kRelayState[] = "RelayState";

Expand Down Expand Up @@ -197,20 +194,10 @@ class SamlTest : public InProcessBrowserTest {
virtual ~SamlTest() {}

virtual void SetUp() OVERRIDE {
// Start embedded test server here so that we can get server base url
// and override Gaia urls in SetupCommandLine.
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());

// Start the GAIA https wrapper here so that the GAIA URLs can be pointed at
// it in SetUpCommandLine().
gaia_https_forwarder_.reset(
new HTTPSForwarder(embedded_test_server()->base_url()));
ASSERT_TRUE(gaia_https_forwarder_->Start());

// Start the SAML IdP https wrapper here so that GAIA can be pointed at it
// in SetUpCommandLine().
saml_https_forwarder_.reset(
new HTTPSForwarder(embedded_test_server()->base_url()));
ASSERT_TRUE(saml_https_forwarder_->Start());

// Stop IO thread here because no threads are allowed while
// spawning sandbox host process. See crbug.com/322732.
embedded_test_server()->StopThread();
Expand All @@ -228,21 +215,28 @@ class SamlTest : public InProcessBrowserTest {
command_line->AppendSwitch(::switches::kDisableBackgroundNetworking);
command_line->AppendSwitchASCII(switches::kLoginProfile, "user");

const GURL gaia_url = gaia_https_forwarder_->GetURL("");
command_line->AppendSwitchASCII(::switches::kGaiaUrl, gaia_url.spec());
command_line->AppendSwitchASCII(::switches::kLsoUrl, gaia_url.spec());
const GURL& server_url = embedded_test_server()->base_url();

std::string gaia_host("gaia");
GURL::Replacements replace_gaia_host;
replace_gaia_host.SetHostStr(gaia_host);
gaia_url_ = server_url.ReplaceComponents(replace_gaia_host);

command_line->AppendSwitchASCII(::switches::kGaiaUrl, gaia_url_.spec());
command_line->AppendSwitchASCII(::switches::kLsoUrl, gaia_url_.spec());
command_line->AppendSwitchASCII(::switches::kGoogleApisUrl,
gaia_url.spec());
gaia_url_.spec());
fake_gaia_.Initialize();

std::string saml_idp_host("saml.idp");
GURL::Replacements replace_saml_idp_host;
replace_saml_idp_host.SetHostStr(saml_idp_host);
GURL saml_idp_url = server_url.ReplaceComponents(replace_saml_idp_host);
saml_idp_url = saml_idp_url.Resolve("/SAML/SSO");

const GURL saml_idp_url = saml_https_forwarder_->GetURL("SAML");
fake_saml_idp_.SetUp(saml_idp_url.path(), gaia_url);
fake_saml_idp_.SetUp(saml_idp_url.path(), gaia_url_);
fake_gaia_.RegisterSamlUser(kFirstSAMLUserEmail, saml_idp_url);
fake_gaia_.RegisterSamlUser(kSecondSAMLUserEmail, saml_idp_url);
fake_gaia_.RegisterSamlUser(
kHTTPSAMLUserEmail,
embedded_test_server()->base_url().Resolve("/SAML"));

fake_gaia_.Initialize();
}

virtual void SetUpOnMainThread() OVERRIDE {
Expand Down Expand Up @@ -370,10 +364,9 @@ class SamlTest : public InProcessBrowserTest {
scoped_ptr<content::WindowedNotificationObserver> login_screen_load_observer_;

private:
GURL gaia_url_;
FakeGaia fake_gaia_;
FakeSamlIdp fake_saml_idp_;
scoped_ptr<HTTPSForwarder> gaia_https_forwarder_;
scoped_ptr<HTTPSForwarder> saml_https_forwarder_;

bool saml_load_injected_;

Expand Down Expand Up @@ -533,17 +526,6 @@ IN_PROC_BROWSER_TEST_F(SamlTest, PasswordConfirmFlow) {
OobeScreenWaiter(OobeDisplay::SCREEN_FATAL_ERROR).Wait();
}

// Verifies that when GAIA attempts to redirect to a SAML IdP served over http,
// not https, the redirect is blocked by CSP and an error message is shown.
IN_PROC_BROWSER_TEST_F(SamlTest, HTTPRedirectDisallowed) {
fake_saml_idp()->SetLoginHTMLTemplate("saml_login.html");

WaitForSigninScreen();
GetLoginDisplay()->ShowSigninScreenForCreds(kHTTPSAMLUserEmail, "");

OobeScreenWaiter(OobeDisplay::SCREEN_FATAL_ERROR).Wait();
}

class SAMLPolicyTest : public SamlTest {
public:
SAMLPolicyTest();
Expand Down
62 changes: 0 additions & 62 deletions chrome/browser/chromeos/login/test/https_forwarder.cc

This file was deleted.

37 changes: 0 additions & 37 deletions chrome/browser/chromeos/login/test/https_forwarder.h

This file was deleted.

Loading

0 comments on commit 563e622

Please sign in to comment.