Skip to content

Commit

Permalink
Ensure that SAML IdPs use https on Chrome OS
Browse files Browse the repository at this point in the history
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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=250114

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@250230 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
bartfab@chromium.org committed Feb 10, 2014
1 parent 25e3234 commit f1b0f81
Show file tree
Hide file tree
Showing 13 changed files with 403 additions and 31 deletions.
9 changes: 9 additions & 0 deletions chrome/browser/chromeos/login/oobe_base_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ 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: 40 additions & 22 deletions chrome/browser/chromeos/login/saml/saml_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#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 @@ -42,6 +43,7 @@
#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 @@ -64,7 +66,8 @@ const char kTestSessionLSIDCookie[] = "fake-session-LSID-cookie";

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

const char kRelayState[] = "RelayState";

Expand Down Expand Up @@ -194,10 +197,20 @@ 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 @@ -215,28 +228,21 @@ class SamlTest : public InProcessBrowserTest {
command_line->AppendSwitch(::switches::kDisableBackgroundNetworking);
command_line->AppendSwitchASCII(switches::kLoginProfile, "user");

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());
const GURL gaia_url = gaia_https_forwarder_->GetURL("");
command_line->AppendSwitchASCII(::switches::kGaiaUrl, gaia_url.spec());
command_line->AppendSwitchASCII(::switches::kLsoUrl, gaia_url.spec());
command_line->AppendSwitchASCII(::switches::kGoogleApisUrl,
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");
gaia_url.spec());

fake_saml_idp_.SetUp(saml_idp_url.path(), gaia_url_);
const GURL saml_idp_url = saml_https_forwarder_->GetURL("SAML");
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 @@ -364,9 +370,10 @@ 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 @@ -526,6 +533,17 @@ 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: 62 additions & 0 deletions chrome/browser/chromeos/login/test/https_forwarder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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 "chrome/browser/chromeos/login/test/https_forwarder.h"

#include "base/base_paths.h"
#include "base/files/file_path.h"
#include "base/path_service.h"
#include "base/values.h"
#include "net/test/python_utils.h"

namespace chromeos {

HTTPSForwarder::HTTPSForwarder(const GURL& forward_target)
: net::LocalTestServer(net::LocalTestServer::TYPE_HTTPS,
net::LocalTestServer::kLocalhost,
base::FilePath()),
forward_target_(forward_target) {
}

HTTPSForwarder::~HTTPSForwarder() {
}

bool HTTPSForwarder::SetPythonPath() const {
if (!net::LocalTestServer::SetPythonPath())
return false;

base::FilePath net_testserver_path;
if (!LocalTestServer::GetTestServerPath(&net_testserver_path))
return false;
AppendToPythonPath(net_testserver_path.DirName());

return true;
}

bool HTTPSForwarder::GetTestServerPath(base::FilePath* testserver_path) const {
base::FilePath source_root_dir;
if (!PathService::Get(base::DIR_SOURCE_ROOT, &source_root_dir))
return false;

*testserver_path = source_root_dir.Append("chrome")
.Append("browser")
.Append("chromeos")
.Append("login")
.Append("test")
.Append("https_forwarder.py");
return true;
}

bool HTTPSForwarder::GenerateAdditionalArguments(
base::DictionaryValue* arguments) const {
base::FilePath source_root_dir;
if (!net::LocalTestServer::GenerateAdditionalArguments(arguments) ||
!PathService::Get(base::DIR_SOURCE_ROOT, &source_root_dir))
return false;

arguments->SetString("forward-target", forward_target_.spec());
return true;
}

} // namespace chromeos
37 changes: 37 additions & 0 deletions chrome/browser/chromeos/login/test/https_forwarder.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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.

#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_TEST_HTTPS_FORWARDER_H_
#define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_HTTPS_FORWARDER_H_

#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "net/test/spawned_test_server/local_test_server.h"
#include "url/gurl.h"

namespace chromeos {

// An https test server that forwards all requests to another server. This
// allows a server that supports http only to be accessed over https.
class HTTPSForwarder : public net::LocalTestServer {
public:
explicit HTTPSForwarder(const GURL& forward_target);
virtual ~HTTPSForwarder();

// net::LocalTestServer:
virtual bool SetPythonPath() const OVERRIDE;
virtual bool GetTestServerPath(
base::FilePath* testserver_path) const OVERRIDE;
virtual bool GenerateAdditionalArguments(
base::DictionaryValue* arguments) const OVERRIDE;

private:
GURL forward_target_;

DISALLOW_COPY_AND_ASSIGN(HTTPSForwarder);
};

} // namespace chromeos

#endif // CHROME_BROWSER_CHROMEOS_LOGIN_TEST_HTTPS_FORWARDER_H_
Loading

0 comments on commit f1b0f81

Please sign in to comment.