Commit 69104663 authored by Alex Ilin's avatar Alex Ilin Committed by Commit Bot

[identity] Clear cookie before showing the legacy consent page

The webview's cookie jar isn't reset between getAuthToken() calls
within a Chrome session. This can lead to sub-optimal experience when
the user sees on the consent page the accounts they were previously
signed in.

This behavior is also problematic if we decide to roll back the new
consent flow experiment. After the experiment rollback, the user
will see all their Chrome accounts whereas Chrome should display only
a primary account.

This CL cleans webview's cookies before starting the legacy consent
flow.

Fixed: 1075929
Change-Id: Ia502ca74d5763c206c680772c43764476b74c5e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2168879
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763445}
parent 9d7d8aba
......@@ -16,9 +16,11 @@
#include "chrome/browser/signin/identity_manager_factory.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/ubertoken_fetcher.h"
#include "content/public/browser/storage_partition.h"
#include "google_apis/gaia/gaia_auth_fetcher.h"
#include "google_apis/gaia/gaia_urls.h"
#include "net/base/escape.h"
#include "services/network/public/mojom/cookie_manager.mojom.h"
namespace extensions {
......@@ -120,6 +122,15 @@ void GaiaWebAuthFlow::OnUbertokenFetchComplete(GoogleServiceAuthError error,
GaiaUrls::GetInstance()->merge_session_url().Resolve(merge_query));
web_flow_ = CreateWebAuthFlow(merge_url);
network::mojom::CookieManager* cookie_manager =
web_flow_->GetGuestPartition()->GetCookieManagerForBrowserProcess();
cookie_manager->DeleteCookies(
network::mojom::CookieDeletionFilter::New(),
base::BindOnce(&GaiaWebAuthFlow::OnCookiesDeleted,
weak_ptr_factory_.GetWeakPtr()));
}
void GaiaWebAuthFlow::OnCookiesDeleted(uint32_t num_deleted) {
web_flow_->Start();
}
......
......@@ -6,6 +6,7 @@
#define CHROME_BROWSER_EXTENSIONS_API_IDENTITY_GAIA_WEB_AUTH_FLOW_H_
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/extensions/api/identity/extension_token_key.h"
#include "chrome/browser/extensions/api/identity/web_auth_flow.h"
#include "extensions/common/manifest_handlers/oauth2_manifest_handler.h"
......@@ -77,6 +78,9 @@ class GaiaWebAuthFlow : public WebAuthFlow::Delegate {
void OnUbertokenFetchComplete(GoogleServiceAuthError error,
const std::string& token);
// DeleteCookies completion callback.
void OnCookiesDeleted(uint32_t num_deleted);
// WebAuthFlow::Delegate implementation.
void OnAuthFlowFailure(WebAuthFlow::Failure failure) override;
void OnAuthFlowURLChange(const GURL& redirect_url) override;
......@@ -96,6 +100,8 @@ class GaiaWebAuthFlow : public WebAuthFlow::Delegate {
std::unique_ptr<signin::UbertokenFetcher> ubertoken_fetcher_;
std::unique_ptr<WebAuthFlow> web_flow_;
base::WeakPtrFactory<GaiaWebAuthFlow> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(GaiaWebAuthFlow);
};
......
......@@ -7,21 +7,44 @@
#include <vector>
#include "base/run_loop.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_storage_partition.h"
#include "services/network/test/test_cookie_manager.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace extensions {
class DeleteCookiesTestCookieManager : public network::TestCookieManager {
public:
void DeleteCookies(network::mojom::CookieDeletionFilterPtr filter,
DeleteCookiesCallback callback) override {
cookies_deleted_ = true;
std::move(callback).Run(0U);
}
bool cookies_deleted() { return cookies_deleted_; }
private:
bool cookies_deleted_ = false;
};
class FakeWebAuthFlow : public WebAuthFlow {
public:
explicit FakeWebAuthFlow(WebAuthFlow::Delegate* delegate)
: WebAuthFlow(delegate,
NULL,
GURL(),
WebAuthFlow::INTERACTIVE) {}
: WebAuthFlow(delegate, nullptr, GURL(), WebAuthFlow::INTERACTIVE) {
storage_partition.set_cookie_manager_for_browser_process(&cookie_manager);
}
void Start() override { EXPECT_TRUE(cookie_manager.cookies_deleted()); }
void Start() override {}
content::StoragePartition* GetGuestPartition() override {
return &storage_partition;
}
private:
content::TestStoragePartition storage_partition;
DeleteCookiesTestCookieManager cookie_manager;
};
class TestGaiaWebAuthFlow : public GaiaWebAuthFlow {
......
......@@ -121,7 +121,7 @@ void WebAuthFlow::DetachDelegateAndDelete() {
base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
}
content::StoragePartition* WebAuthFlow::GetGuestPartition() const {
content::StoragePartition* WebAuthFlow::GetGuestPartition() {
return content::BrowserContext::GetStoragePartitionForSite(
profile_, GetWebViewSiteURL());
}
......
......@@ -89,8 +89,8 @@ class WebAuthFlow : public content::NotificationObserver,
void DetachDelegateAndDelete();
// Returns a StoragePartition of the guest webview. Used to inject cookies
// into Gaia page.
content::StoragePartition* GetGuestPartition() const;
// into Gaia page. Can override for testing.
virtual content::StoragePartition* GetGuestPartition();
// Returns an ID string attached to the window. Can override for testing.
virtual const std::string& GetAppWindowKey() const;
......
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