Skip to content

Commit

Permalink
[Extension Auth] Add infobar to opened tab from auth
Browse files Browse the repository at this point in the history
Added an infobar when using WebAuthFlow with a browser tab, to display
to the user that this tab was opened by a specific extension for
authentication purposes.
Extension name is visible in the message displayed.
Message displayed is not final yet and will be updated in a future CL as a translation string.

Change-Id: I5b95567d55834aff4097d698ee67aecc5ad9224c
Bug: 1408402
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4165838
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Ryan Sultanem <rsult@google.com>
Cr-Commit-Position: refs/heads/main@{#1096098}
  • Loading branch information
Ryan Sultanem authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent 2cc8d5a commit d0115fe
Show file tree
Hide file tree
Showing 16 changed files with 240 additions and 24 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ static_library("extensions") {
"api/identity/identity_token_cache.h",
"api/identity/web_auth_flow.cc",
"api/identity/web_auth_flow.h",
"api/identity/web_auth_flow_info_bar_delegate.cc",
"api/identity/web_auth_flow_info_bar_delegate.h",
"api/idltest/idltest_api.cc",
"api/idltest/idltest_api.h",
"api/image_writer_private/destroy_partitions_operation.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ GaiaRemoteConsentFlow::GaiaRemoteConsentFlow(
Delegate* delegate,
Profile* profile,
const ExtensionTokenKey& token_key,
const RemoteConsentResolutionData& resolution_data)
const RemoteConsentResolutionData& resolution_data,
const std::string& extension_name)
: delegate_(delegate),
profile_(profile),
account_id_(token_key.account_info.account_id),
resolution_data_(resolution_data),
extension_name_(extension_name),
web_flow_started_(false) {}

GaiaRemoteConsentFlow::~GaiaRemoteConsentFlow() {
Expand All @@ -67,7 +69,7 @@ void GaiaRemoteConsentFlow::Start() {
if (!web_flow_) {
web_flow_ = std::make_unique<WebAuthFlow>(
this, profile_, resolution_data_.url, WebAuthFlow::INTERACTIVE,
WebAuthFlow::GET_AUTH_TOKEN);
WebAuthFlow::GET_AUTH_TOKEN, extension_name_);
#if BUILDFLAG(IS_CHROMEOS_LACROS)
// `profile_` may be nullptr in tests.
if (profile_) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ class GaiaRemoteConsentFlow
GaiaRemoteConsentFlow(Delegate* delegate,
Profile* profile,
const ExtensionTokenKey& token_key,
const RemoteConsentResolutionData& resolution_data);
const RemoteConsentResolutionData& resolution_data,
const std::string& extension_name);
~GaiaRemoteConsentFlow() override;

GaiaRemoteConsentFlow(const GaiaRemoteConsentFlow& other) = delete;
Expand Down Expand Up @@ -98,6 +99,7 @@ class GaiaRemoteConsentFlow
raw_ptr<Profile> profile_;
CoreAccountId account_id_;
RemoteConsentResolutionData resolution_data_;
const std::string extension_name_;

std::unique_ptr<WebAuthFlow> web_flow_;
bool web_flow_started_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ class GaiaRemoteConsentFlowBrowserTest : public InProcessBrowserTest {
RemoteConsentResolutionData resolution_data;
resolution_data.url = fake_gaia_test_server()->GetURL("/title1.html");

flow_ = std::make_unique<GaiaRemoteConsentFlow>(&mock(), profile(),
token_key, resolution_data);
flow_ = std::make_unique<GaiaRemoteConsentFlow>(
&mock(), profile(), token_key, resolution_data, "extension_name");

content::TestNavigationObserver navigation_observer(resolution_data.url);
navigation_observer.StartWatchingNewWebContents();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class FakeWebAuthFlowWithWindowKey : public WebAuthFlow {
nullptr,
GURL(),
WebAuthFlow::INTERACTIVE,
WebAuthFlow::GET_AUTH_TOKEN),
WebAuthFlow::GET_AUTH_TOKEN,
"extension_name"),
fake_window_key_(window_key) {}

~FakeWebAuthFlowWithWindowKey() override = default;
Expand All @@ -52,7 +53,11 @@ class TestGaiaRemoteConsentFlow : public GaiaRemoteConsentFlow {
const ExtensionTokenKey& token_key,
const RemoteConsentResolutionData& resolution_data,
const std::string& window_key)
: GaiaRemoteConsentFlow(delegate, nullptr, token_key, resolution_data),
: GaiaRemoteConsentFlow(delegate,
nullptr,
token_key,
resolution_data,
"extension_name"),
window_key_(window_key) {
SetWebAuthFlowForTesting(
std::make_unique<FakeWebAuthFlowWithWindowKey>(this, window_key_));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ void IdentityGetAuthTokenFunction::ShowExtensionLoginPrompt() {
void IdentityGetAuthTokenFunction::ShowRemoteConsentDialog(
const RemoteConsentResolutionData& resolution_data) {
gaia_remote_consent_flow_ = std::make_unique<GaiaRemoteConsentFlow>(
this, GetProfile(), token_key_, resolution_data);
this, GetProfile(), token_key_, resolution_data, extension()->name());
gaia_remote_consent_flow_->Start();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ ExtensionFunction::ResponseAction IdentityLaunchWebAuthFlowFunction::Run() {
AddRef(); // Balanced in OnAuthFlowSuccess/Failure.

auth_flow_ = std::make_unique<WebAuthFlow>(this, profile, auth_url, mode,
WebAuthFlow::LAUNCH_WEB_AUTH_FLOW);
WebAuthFlow::LAUNCH_WEB_AUTH_FLOW,
extension()->name());
auth_flow_->Start();
return RespondLater();
}
Expand Down
43 changes: 37 additions & 6 deletions chrome/browser/extensions/api/identity/web_auth_flow.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/task/single_thread_task_runner.h"
#include "base/trace_event/trace_event.h"
#include "chrome/browser/extensions/api/identity/web_auth_flow_info_bar_delegate.h"
#include "chrome/browser/extensions/component_loader.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h"
Expand Down Expand Up @@ -98,12 +99,14 @@ WebAuthFlow::WebAuthFlow(Delegate* delegate,
Profile* profile,
const GURL& provider_url,
Mode mode,
Partition partition)
Partition partition,
const std::string& extension_name)
: delegate_(delegate),
profile_(profile),
provider_url_(provider_url),
mode_(mode),
partition_(partition) {
partition_(partition),
extension_name_(extension_name) {
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("identity", "WebAuthFlow", this);
}

Expand All @@ -114,6 +117,8 @@ WebAuthFlow::~WebAuthFlow() {
web_contents()->Close();
}

CloseInfoBar();

// Stop listening to notifications first since some of the code
// below may generate notifications.
WebContentsObserver::Observe(nullptr);
Expand Down Expand Up @@ -244,6 +249,20 @@ bool WebAuthFlow::IsObservingProviderWebContents() const {
(embedded_window_created_ || using_auth_with_browser_tab_);
}

void WebAuthFlow::DisplayInfoBar() {
DCHECK(web_contents());
DCHECK(using_auth_with_browser_tab_);

info_bar_delegate_ =
WebAuthFlowInfoBarDelegate::Create(web_contents(), extension_name_);
}

void WebAuthFlow::CloseInfoBar() {
if (info_bar_delegate_) {
info_bar_delegate_->CloseInfoBar();
}
}

void WebAuthFlow::BeforeUrlLoaded(const GURL& url) {
if (delegate_ && IsObservingProviderWebContents())
delegate_->OnAuthFlowURLChange(url);
Expand All @@ -263,6 +282,8 @@ void WebAuthFlow::AfterUrlLoaded() {
NavigateParams params(browser_displayer.browser(),
std::move(web_contents_));
Navigate(&params);

DisplayInfoBar();
}
}

Expand Down Expand Up @@ -306,22 +327,26 @@ void WebAuthFlow::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
// If web_contents_ is nullptr, then the auth page tab is opened.
// If the navigation is initiated by the user, the tab will exit the auth
// flow screen, this should result in a declined authentication.
// flow screen, this should result in a declined authentication and deleting
// the flow.
if (using_auth_with_browser_tab_ && !web_contents_ &&
!navigation_handle->IsRendererInitiated()) {
// Stop observing the web contents since it is not part of the flow anymore.
WebContentsObserver::Observe(nullptr);
delegate_->OnAuthFlowFailure(Failure::USER_NAVIGATED_AWAY);
return;
}

if (navigation_handle->IsInPrimaryMainFrame())
if (navigation_handle->IsInPrimaryMainFrame()) {
BeforeUrlLoaded(navigation_handle->GetURL());
}
}

void WebAuthFlow::DidRedirectNavigation(
content::NavigationHandle* navigation_handle) {
if (navigation_handle->IsInPrimaryMainFrame())
if (navigation_handle->IsInPrimaryMainFrame()) {
BeforeUrlLoaded(navigation_handle->GetURL());
}
}

void WebAuthFlow::DidFinishNavigation(
Expand Down Expand Up @@ -373,8 +398,14 @@ void WebAuthFlow::DidFinishNavigation(
navigation_handle->GetResponseHeaders()->response_code());
}

if (failed && delegate_)
if (failed && delegate_) {
delegate_->OnAuthFlowFailure(LOAD_FAILED);
}
}

base::WeakPtr<WebAuthFlowInfoBarDelegate>
WebAuthFlow::GetInfoBarDelegateForTesting() {
return info_bar_delegate_;
}

} // namespace extensions
16 changes: 15 additions & 1 deletion chrome/browser/extensions/api/identity/web_auth_flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/feature_list.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "content/public/browser/storage_partition_config.h"
#include "content/public/browser/web_contents_observer.h"
#include "extensions/browser/app_window/app_window_registry.h"
Expand All @@ -24,6 +25,8 @@ class StoragePartition;

namespace extensions {

class WebAuthFlowInfoBarDelegate;

// When enabled, cookies in the `launchWebAuthFlow()` partition are persisted
// across browser restarts.
BASE_DECLARE_FEATURE(kPersistentStorageForWebAuthFlow);
Expand Down Expand Up @@ -86,7 +89,8 @@ class WebAuthFlow : public content::WebContentsObserver,
Profile* profile,
const GURL& provider_url,
Mode mode,
Partition partition);
Partition partition,
const std::string& extension_name);

WebAuthFlow(const WebAuthFlow&) = delete;
WebAuthFlow& operator=(const WebAuthFlow&) = delete;
Expand All @@ -112,6 +116,9 @@ class WebAuthFlow : public content::WebContentsObserver,
Partition partition,
content::BrowserContext* browser_context);

// Returns nullptr if the InfoBar is not displayed.
base::WeakPtr<WebAuthFlowInfoBarDelegate> GetInfoBarDelegateForTesting();

private:
friend class ::WebAuthFlowTest;

Expand Down Expand Up @@ -139,6 +146,9 @@ class WebAuthFlow : public content::WebContentsObserver,

bool IsObservingProviderWebContents() const;

void DisplayInfoBar();
void CloseInfoBar();

raw_ptr<Delegate> delegate_ = nullptr;
const raw_ptr<Profile> profile_;
const GURL provider_url_;
Expand All @@ -159,6 +169,10 @@ class WebAuthFlow : public content::WebContentsObserver,
// `this`. When this value becomes nullptr, this means that the browser tab
// has taken ownership and the interactive tab was opened.
std::unique_ptr<content::WebContents> web_contents_;
const std::string extension_name_;
// WeakPtr to the info bar delegate attached to the auth tab when opened. Used
// to close the info bar when closing the flow if still valid.
base::WeakPtr<WebAuthFlowInfoBarDelegate> info_bar_delegate_ = nullptr;
};

} // namespace extensions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
#include "chrome/browser/extensions/api/identity/web_auth_flow.h"

#include "base/strings/strcat.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
#include "chrome/browser/extensions/api/identity/web_auth_flow_info_bar_delegate.h"
#include "chrome/browser/profiles/keep_alive/profile_keep_alive_types.h"
#include "chrome/browser/profiles/keep_alive/scoped_profile_keep_alive.h"
#include "chrome/browser/ui/browser.h"
Expand All @@ -26,6 +28,8 @@

namespace extensions {

const char kExtensionName[] = "extension_name";

class MockWebAuthFlowDelegate : public WebAuthFlow::Delegate {
public:
MOCK_METHOD(void, OnAuthFlowURLChange, (const GURL&), (override));
Expand All @@ -38,12 +42,25 @@ class WebAuthFlowBrowserTest : public InProcessBrowserTest {
void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();
ASSERT_TRUE(embedded_test_server()->Start());

// Delete the flow early if OnAuthFlowFailure is called. Simulates real
// usages.
ON_CALL(mock(), OnAuthFlowFailure(testing::_))
.WillByDefault(
[this](WebAuthFlow::Failure failure) { DeleteWebAuthFlow(); });
}

void TearDownOnMainThread() override {
void DeleteWebAuthFlow() {
DCHECK(web_auth_flow_);
// Delete the web auth flow (uses DeleteSoon).
web_auth_flow_.release()->DetachDelegateAndDelete();
base::RunLoop().RunUntilIdle();
}

void TearDownOnMainThread() override {
if (web_auth_flow_) {
DeleteWebAuthFlow();
}
InProcessBrowserTest::TearDownOnMainThread();
}

Expand All @@ -55,15 +72,18 @@ class WebAuthFlowBrowserTest : public InProcessBrowserTest {
if (!profile)
profile = browser()->profile();

web_auth_flow_ = std::make_unique<WebAuthFlow>(
&mock_web_auth_flow_delegate_, profile, url, mode, partition);
web_auth_flow_ =
std::make_unique<WebAuthFlow>(&mock_web_auth_flow_delegate_, profile,
url, mode, partition, kExtensionName);
web_auth_flow_->Start();
}

WebAuthFlow* web_auth_flow() { return web_auth_flow_.get(); }

content::WebContents* web_contents() {
DCHECK(web_auth_flow_);
if (!web_auth_flow_) {
return nullptr;
}
return web_auth_flow_->web_contents();
}

Expand Down Expand Up @@ -353,6 +373,16 @@ IN_PROC_BROWSER_TEST_F(WebAuthFlowWithBrowserTabBrowserTest,
EXPECT_EQ(tabs->count(), initial_tab_count + 1);
EXPECT_EQ(tabs->GetActiveWebContents()->GetLastCommittedURL(), auth_url);

// Check info bar exists and displays proper message with extension name.
base::WeakPtr<WebAuthFlowInfoBarDelegate> infobar_delegate =
web_auth_flow()->GetInfoBarDelegateForTesting();
EXPECT_TRUE(infobar_delegate);
EXPECT_EQ(
infobar_delegate->GetIdentifier(),
infobars::InfoBarDelegate::EXTENSIONS_WEB_AUTH_FLOW_INFOBAR_DELEGATE);
EXPECT_TRUE(infobar_delegate->GetMessageText().find(
base::UTF8ToUTF16(std::string(kExtensionName))));

//---------------------------------------------------------------------
// Part of the test that closes the tab, simulating declining the consent.
//---------------------------------------------------------------------
Expand Down Expand Up @@ -382,6 +412,10 @@ IN_PROC_BROWSER_TEST_F(
//---------------------------------------------------------------------
testing::Mock::VerifyAndClearExpectations(&mock());

// Keeping a reference to the info bar delegate to check later.
base::WeakPtr<WebAuthFlowInfoBarDelegate> auth_info_bar =
web_auth_flow()->GetInfoBarDelegateForTesting();

GURL new_url = embedded_test_server()->GetURL("a.com", "/new.html");
EXPECT_CALL(mock(),
OnAuthFlowFailure(WebAuthFlow::Failure::USER_NAVIGATED_AWAY));
Expand All @@ -392,11 +426,13 @@ IN_PROC_BROWSER_TEST_F(
web_contents()->GetController().LoadURLWithParams(load_params);
web_contents_observer.Wait();

// New tab is not execpted to be closed, it is now used for navigation and not
// New tab is not expected to be closed, it is now used for navigation and not
// part of the flow anymore.
EXPECT_EQ(web_contents(), nullptr);
EXPECT_EQ(tabs->count(), initial_tab_count + 1);
EXPECT_EQ(tabs->GetActiveWebContents()->GetLastCommittedURL(), new_url);
// Infobar should be closed on navigation.
EXPECT_FALSE(auth_info_bar);
}

IN_PROC_BROWSER_TEST_F(WebAuthFlowWithBrowserTabBrowserTest,
Expand Down

0 comments on commit d0115fe

Please sign in to comment.