From 9a61498131711940c5bbbd54051682dee41b012f Mon Sep 17 00:00:00 2001 From: Mikel Astiz Date: Wed, 28 Apr 2021 11:02:39 +0000 Subject: [PATCH] [signin] Add browser tests for Google-Accounts-RemoveLocalAccount This patch extends FakeGaia with a page that browser tests can visit to trigger the HTTP header Google-Accounts-RemoveLocalAccount. Browser tests are added to leverage this functionality and verify that IdentityManager notifies observers appropriately via OnAccountsInCookieUpdated(). Change-Id: I9a6ea41cecabbc094f2155d3b6df61ec74941525 Bug: 1078762 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2842011 Commit-Queue: Mikel Astiz Reviewed-by: David Roger Cr-Commit-Position: refs/heads/master@{#876994} --- .../remove_local_account_browsertest.cc | 142 ++++++++++++++++++ chrome/test/BUILD.gn | 1 + google_apis/gaia/fake_gaia.cc | 71 ++++++++- google_apis/gaia/fake_gaia.h | 11 ++ 4 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 chrome/browser/signin/remove_local_account_browsertest.cc diff --git a/chrome/browser/signin/remove_local_account_browsertest.cc b/chrome/browser/signin/remove_local_account_browsertest.cc new file mode 100644 index 00000000000000..7ded8336adaea5 --- /dev/null +++ b/chrome/browser/signin/remove_local_account_browsertest.cc @@ -0,0 +1,142 @@ +// Copyright 2021 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 + +#include "base/run_loop.h" +#include "base/test/bind.h" +#include "build/build_config.h" +#include "chrome/browser/signin/identity_manager_factory.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_tabstrip.h" +#include "chrome/test/base/mixin_based_in_process_browser_test.h" +#include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h" +#include "components/signin/public/identity_manager/identity_manager.h" +#include "components/signin/public/identity_manager/identity_test_utils.h" +#include "components/signin/public/identity_manager/test_identity_manager_observer.h" +#include "content/public/test/browser_test.h" +#include "google_apis/gaia/fake_gaia.h" +#include "google_apis/gaia/gaia_switches.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "net/test/embedded_test_server/http_response.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +#if BUILDFLAG(IS_CHROMEOS_ASH) +#include "chrome/browser/ash/login/test/network_portal_detector_mixin.h" +#endif + +namespace { + +using testing::Contains; +using testing::Not; + +MATCHER_P(ListedAccountMatchesGaiaId, gaia_id, "") { + return arg.gaia_id == std::string(gaia_id); +} + +const char kTestGaiaId[] = "123"; + +class RemoveLocalAccountTest : public MixinBasedInProcessBrowserTest { + protected: + RemoveLocalAccountTest() + : embedded_test_server_(net::EmbeddedTestServer::TYPE_HTTPS) { + embedded_test_server_.RegisterRequestHandler(base::BindRepeating( + &FakeGaia::HandleRequest, base::Unretained(&fake_gaia_))); + } + + ~RemoveLocalAccountTest() override = default; + + signin::IdentityManager* identity_manager() { + return IdentityManagerFactory::GetForProfile(browser()->profile()); + } + + signin::AccountsInCookieJarInfo WaitUntilAccountsInCookieUpdated() { + signin::TestIdentityManagerObserver observer(identity_manager()); + base::RunLoop run_loop; + observer.SetOnAccountsInCookieUpdatedCallback(run_loop.QuitClosure()); + run_loop.Run(); + return observer.AccountsInfoFromAccountsInCookieUpdatedCallback(); + } + + // MixinBasedInProcessBrowserTest: + void SetUpCommandLine(base::CommandLine* command_line) override { + MixinBasedInProcessBrowserTest::SetUpCommandLine(command_line); + ASSERT_TRUE(embedded_test_server_.InitializeAndListen()); + const GURL base_url = embedded_test_server_.base_url(); + command_line->AppendSwitchASCII(switches::kGaiaUrl, base_url.spec()); + } + + void SetUpOnMainThread() override { + MixinBasedInProcessBrowserTest::SetUpOnMainThread(); + fake_gaia_.Initialize(); + + FakeGaia::MergeSessionParams params; + params.signed_out_gaia_ids.push_back(kTestGaiaId); + fake_gaia_.UpdateMergeSessionParams(params); + + embedded_test_server_.StartAcceptingConnections(); + +#if BUILDFLAG(IS_CHROMEOS_ASH) + // ChromeSigninClient uses chromeos::DelayNetworkCall() which requires + // simulating being online. + network_portal_detector_.SimulateDefaultNetworkState( + ash::NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE); +#endif + } + + FakeGaia fake_gaia_; + net::EmbeddedTestServer embedded_test_server_; + +#if BUILDFLAG(IS_CHROMEOS_ASH) + ash::NetworkPortalDetectorMixin network_portal_detector_{&mixin_host_}; +#endif +}; + +IN_PROC_BROWSER_TEST_F(RemoveLocalAccountTest, ShouldNotifyObservers) { + // To enforce an initial ListAccounts fetch and the corresponding notification + // to observers, make the current list as stale. This is done for the purpose + // of documenting assertions on the AccountsInCookieJarInfo passed to + // observers during notification. + signin::SetFreshnessOfAccountsInGaiaCookie(identity_manager(), + /*accounts_are_fresh=*/false); + + ASSERT_FALSE(identity_manager()->GetAccountsInCookieJar().accounts_are_fresh); + const signin::AccountsInCookieJarInfo + cookie_jar_info_in_initial_notification = + WaitUntilAccountsInCookieUpdated(); + ASSERT_TRUE(cookie_jar_info_in_initial_notification.accounts_are_fresh); + ASSERT_THAT(cookie_jar_info_in_initial_notification.signed_out_accounts, + Contains(ListedAccountMatchesGaiaId(kTestGaiaId))); + + const signin::AccountsInCookieJarInfo initial_cookie_jar_info = + identity_manager()->GetAccountsInCookieJar(); + ASSERT_TRUE(initial_cookie_jar_info.accounts_are_fresh); + ASSERT_THAT(initial_cookie_jar_info.signed_out_accounts, + Contains(ListedAccountMatchesGaiaId(kTestGaiaId))); + + // Open a FakeGaia page that issues the desired HTTP response header with + // Google-Accounts-RemoveLocalAccount. + chrome::AddTabAt(browser(), + fake_gaia_.GetDummyRemoveLocalAccountURL(kTestGaiaId), + /*index=*/0, + /*foreground=*/true); + + // Wait until observers are notified with OnAccountsInCookieUpdated(). + const signin::AccountsInCookieJarInfo + cookie_jar_info_in_updated_notification = + WaitUntilAccountsInCookieUpdated(); + + EXPECT_TRUE(cookie_jar_info_in_updated_notification.accounts_are_fresh); + EXPECT_THAT(cookie_jar_info_in_updated_notification.signed_out_accounts, + Not(Contains(ListedAccountMatchesGaiaId(kTestGaiaId)))); + + const signin::AccountsInCookieJarInfo updated_cookie_jar_info = + identity_manager()->GetAccountsInCookieJar(); + EXPECT_TRUE(updated_cookie_jar_info.accounts_are_fresh); + EXPECT_THAT(updated_cookie_jar_info.signed_out_accounts, + Not(Contains(ListedAccountMatchesGaiaId(kTestGaiaId)))); +} + +} // namespace diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 9c1bf5d0b67b01..c8022e41f24d3c 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -1386,6 +1386,7 @@ if (!is_android) { "../browser/signin/e2e_tests/live_sign_in_test.cc", "../browser/signin/e2e_tests/live_test.cc", "../browser/signin/e2e_tests/live_test.h", + "../browser/signin/remove_local_account_browsertest.cc", "../browser/site_isolation/chrome_site_per_process_browsertest.cc", "../browser/site_isolation/chrome_site_per_process_test.cc", "../browser/site_isolation/chrome_site_per_process_test.h", diff --git a/google_apis/gaia/fake_gaia.cc b/google_apis/gaia/fake_gaia.cc index bacf4c0e649734..ec154d1a3f4a98 100644 --- a/google_apis/gaia/fake_gaia.cc +++ b/google_apis/gaia/fake_gaia.cc @@ -63,6 +63,7 @@ const char kTestCookieAttributes[] = "; Path=/; HttpOnly; SameSite=None; Secure"; const char kDefaultGaiaId[] = "12345"; +const char kDefaultEmail[] = "email12345@foo.com"; const base::FilePath::CharType kEmbeddedSetupChromeos[] = FILE_PATH_LITERAL("google_apis/test/embedded_setup_chromeos.html"); @@ -71,9 +72,11 @@ const base::FilePath::CharType kEmbeddedSetupChromeos[] = const char kAuthHeaderBearer[] = "Bearer "; const char kAuthHeaderOAuth[] = "OAuth "; -const char kListAccountsResponseFormat[] = - "[\"gaia.l.a.r\",[[\"gaia.l.a\",1,\"\",\"%s\",\"\",1,1,0,0,1,\"12345\"]]]"; +const char kIndividualListedAccountResponseFormat[] = + "[\"gaia.l.a\",1,\"\",\"%s\",\"\",1,1,0,0,1,\"%s\",11,12,13,%d]"; +const char kListAccountsResponseFormat[] = "[\"gaia.l.a.r\",[%s]]"; +const char kDummyRemoveLocalAccountPath[] = "DummyRemoveLocalAccount"; const char kDummySAMLContinuePath[] = "DummySAMLContinue"; typedef std::map CookieMap; @@ -185,6 +188,9 @@ void FakeGaia::MergeSessionParams::Update(const MergeSessionParams& update) { maybe_update_field(&MergeSessionParams::session_sid_cookie); maybe_update_field(&MergeSessionParams::session_lsid_cookie); maybe_update_field(&MergeSessionParams::email); + + if (!update.signed_out_gaia_ids.empty()) + signed_out_gaia_ids = update.signed_out_gaia_ids; } FakeGaia::SyncTrustedVaultKeys::SyncTrustedVaultKeys() = default; @@ -247,6 +253,14 @@ std::string FakeGaia::GetGaiaIdOfEmail(const std::string& email) const { it->second; } +std::string FakeGaia::GetEmailOfGaiaId(const std::string& gaia_id) const { + for (const auto& email_and_gaia_id : email_to_gaia_id_map_) { + if (email_and_gaia_id.second == gaia_id) + return email_and_gaia_id.first; + } + return kDefaultEmail; +} + void FakeGaia::AddGoogleAccountsSigninHeader(BasicHttpResponse* http_response, const std::string& email) const { DCHECK(http_response); @@ -360,6 +374,11 @@ void FakeGaia::Initialize() { // Handles ReAuth API token fetch call. REGISTER_RESPONSE_HANDLER(gaia_urls->reauth_api_url(), HandleGetReAuthProofToken); + + // Handles API for browser tests to manually remove local accounts. + REGISTER_RESPONSE_HANDLER( + gaia_urls->gaia_url().Resolve(kDummyRemoveLocalAccountPath), + HandleDummyRemoveLocalAccount); } FakeGaia::RequestHandlerMap::iterator FakeGaia::FindHandlerByPathPrefix( @@ -442,6 +461,12 @@ void FakeGaia::SetErrorResponse(const GURL& gaia_url, error_responses_[gaia_url.path()] = http_status_code; } +GURL FakeGaia::GetDummyRemoveLocalAccountURL(const std::string& gaia_id) const { + GURL url = + GaiaUrls::GetInstance()->gaia_url().Resolve(kDummyRemoveLocalAccountPath); + return net::AppendQueryParameter(url, "gaia_id", gaia_id); +} + void FakeGaia::SetRefreshTokenToDeviceIdMap( const RefreshTokenToDeviceIdMap& refresh_token_to_device_id_map) { refresh_token_to_device_id_map_ = refresh_token_to_device_id_map; @@ -859,8 +884,26 @@ void FakeGaia::HandleIssueToken(const HttpRequest& request, void FakeGaia::HandleListAccounts(const HttpRequest& request, BasicHttpResponse* http_response) { - http_response->set_content(base::StringPrintf( - kListAccountsResponseFormat, merge_session_params_.email.c_str())); + const int kAccountIsSignedIn = 0; + const int kAccountIsSignedOut = 1; + + std::vector listed_accounts; + listed_accounts.push_back(base::StringPrintf( + kIndividualListedAccountResponseFormat, + merge_session_params_.email.c_str(), kDefaultGaiaId, kAccountIsSignedIn)); + + for (const std::string& gaia_id : merge_session_params_.signed_out_gaia_ids) { + DCHECK_NE(kDefaultGaiaId, gaia_id); + + const std::string email = GetEmailOfGaiaId(gaia_id); + listed_accounts.push_back(base::StringPrintf( + kIndividualListedAccountResponseFormat, email.c_str(), gaia_id.c_str(), + kAccountIsSignedOut)); + } + + http_response->set_content( + base::StringPrintf(kListAccountsResponseFormat, + base::JoinString(listed_accounts, ",").c_str())); http_response->set_code(net::HTTP_OK); } @@ -1004,6 +1047,26 @@ void FakeGaia::HandleMultilogin(const HttpRequest& request, http_response->set_code(net::HTTP_OK); } +void FakeGaia::HandleDummyRemoveLocalAccount( + const net::test_server::HttpRequest& request, + net::test_server::BasicHttpResponse* http_response) { + DCHECK(http_response); + + std::string gaia_id; + GetQueryParameter(request.GetURL().query(), "gaia_id", &gaia_id); + + if (!base::Erase(merge_session_params_.signed_out_gaia_ids, gaia_id)) { + http_response->set_code(net::HTTP_BAD_REQUEST); + return; + } + + http_response->AddCustomHeader( + "Google-Accounts-RemoveLocalAccount", + base::StringPrintf("obfuscatedid=\"%s\"", gaia_id.c_str())); + http_response->set_content(""); + http_response->set_code(net::HTTP_OK); +} + std::string FakeGaia::GetEmbeddedSetupChromeosResponseContent() const { if (embedded_setup_chromeos_iframe_url_.is_empty()) return embedded_setup_chromeos_response_; diff --git a/google_apis/gaia/fake_gaia.h b/google_apis/gaia/fake_gaia.h index 4b5b250d0908c5..92d47d0e606413 100644 --- a/google_apis/gaia/fake_gaia.h +++ b/google_apis/gaia/fake_gaia.h @@ -85,6 +85,9 @@ class FakeGaia { // The e-mail address returned by /ListAccounts. std::string email; + + // List of signed out gaia IDs returned by /ListAccounts. + std::vector signed_out_gaia_ids; }; struct SyncTrustedVaultKeys { @@ -194,6 +197,10 @@ class FakeGaia { // Returns the is_device_owner param from the reauth URL if any. const std::string& is_device_owner() { return is_device_owner_; } + // Returns the fake server's URL that browser tests can visit to trigger a + // RemoveLocalAccount event. + GURL GetDummyRemoveLocalAccountURL(const std::string& gaia_id) const; + protected: // HTTP handler for /MergeSession. virtual void HandleMergeSession( @@ -209,6 +216,7 @@ class FakeGaia { std::map; std::string GetGaiaIdOfEmail(const std::string& email) const; + std::string GetEmailOfGaiaId(const std::string& email) const; void AddGoogleAccountsSigninHeader( net::test_server::BasicHttpResponse* http_response, @@ -298,6 +306,9 @@ class FakeGaia { // HTTP handler for /OAuth/Multilogin. void HandleMultilogin(const net::test_server::HttpRequest& request, net::test_server::BasicHttpResponse* http_response); + void HandleDummyRemoveLocalAccount( + const net::test_server::HttpRequest& request, + net::test_server::BasicHttpResponse* http_response); // Returns the access token associated with |auth_token| that matches the // given |client_id| and |scope_string|. If |scope_string| is empty, the first