Skip to content

Commit

Permalink
Connect ApcClient to PasswordsClientUiDelegate.
Browse files Browse the repository at this point in the history
Now that ApcClient is part of the code base, connect it to the
controller method that launches automated password change flows.

Bug: 1321500
Change-Id: I404ea4f6515a25288c5fa1a8fd55aebf3efa40ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629366
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Keitel <jkeitel@google.com>
Cr-Commit-Position: refs/heads/main@{#1000995}
  • Loading branch information
Jan Keitel authored and Chromium LUCI CQ committed May 9, 2022
1 parent b0a5a19 commit 9dbe052
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ void ChromePasswordManagerClient::AutofillHttpAuth(

void ChromePasswordManagerClient::NotifyUserCredentialsWereLeaked(
password_manager::CredentialLeakType leak_type,
const GURL& origin,
const GURL& url,
const std::u16string& username) {
if (GetPasswordFeatureManager()->IsGenerationEnabled() &&
password_manager::features::IsPasswordScriptsFetchingEnabled()) {
Expand All @@ -634,13 +634,13 @@ void ChromePasswordManagerClient::NotifyUserCredentialsWereLeaked(
}

(new CredentialLeakControllerAndroid(
leak_type, origin, username, GetPasswordChangeSuccessTracker(),
leak_type, url, username, GetPasswordChangeSuccessTracker(),
web_contents()->GetTopLevelNativeWindow()))
->ShowDialog();
#else // !BUILDFLAG(IS_ANDROID)
PasswordsClientUIDelegate* manage_passwords_ui_controller =
PasswordsClientUIDelegateFromWebContents(web_contents());
manage_passwords_ui_controller->OnCredentialLeak(leak_type, origin);
manage_passwords_ui_controller->OnCredentialLeak(leak_type, url, username);
#endif // BUILDFLAG(IS_ANDROID)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ class ChromePasswordManagerClient
const password_manager::PasswordFormManagerForUI* form_manager) override;
void NotifyUserCredentialsWereLeaked(
password_manager::CredentialLeakType leak_type,
const GURL& origin,
const GURL& url,
const std::u16string& username) override;
void TriggerReauthForPrimaryAccount(
signin_metrics::ReauthAccessPoint access_point,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ using password_manager::metrics_util::LogLeakDialogTypeAndDismissalReason;

CredentialLeakDialogControllerImpl::CredentialLeakDialogControllerImpl(
PasswordsLeakDialogDelegate* delegate,
CredentialLeakType leak_type)
CredentialLeakType leak_type,
const GURL& url,
const std::u16string& username)
: delegate_(delegate),
leak_type_(leak_type),
leak_dialog_traits_(CreateDialogTraits(leak_type)) {}
leak_dialog_traits_(CreateDialogTraits(leak_type)),
url_(url),
username_(username) {}

CredentialLeakDialogControllerImpl::~CredentialLeakDialogControllerImpl() {
ResetDialog();
Expand All @@ -45,7 +49,7 @@ void CredentialLeakDialogControllerImpl::OnCancelDialog() {

void CredentialLeakDialogControllerImpl::OnAcceptDialog() {
if (ShouldOfferAutomatedPasswordChange()) {
delegate_->StartAutomatedPasswordChange();
delegate_->StartAutomatedPasswordChange(url_, username_);
LogLeakDialogTypeAndDismissalReason(
password_manager::GetLeakDialogType(leak_type_),
LeakDialogDismissalReason::kClickedChangePasswordAutomatically);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ class CredentialLeakDialogControllerImpl
public:
CredentialLeakDialogControllerImpl(
PasswordsLeakDialogDelegate* delegate,
password_manager::CredentialLeakType leak_type);
password_manager::CredentialLeakType leak_type,
const GURL& url,
const std::u16string& username);

CredentialLeakDialogControllerImpl(
const CredentialLeakDialogControllerImpl&) = delete;
Expand Down Expand Up @@ -50,6 +52,8 @@ class CredentialLeakDialogControllerImpl
raw_ptr<PasswordsLeakDialogDelegate> delegate_;
const password_manager::CredentialLeakType leak_type_;
std::unique_ptr<password_manager::LeakDialogTraits> leak_dialog_traits_;
GURL url_;
std::u16string username_;
};

#endif // CHROME_BROWSER_UI_PASSWORDS_CREDENTIAL_LEAK_DIALOG_CONTROLLER_IMPL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "components/password_manager/core/common/password_manager_features.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

namespace {

Expand All @@ -25,6 +26,9 @@ using password_manager::IsSyncing;
using password_manager::metrics_util::LeakDialogDismissalReason;
using testing::StrictMock;

constexpr char kUrl[] = "https://www.example.co.uk";
constexpr char16_t kUsername[] = u"Jane";

class MockCredentialLeakPrompt : public CredentialLeakPrompt {
public:
MockCredentialLeakPrompt() = default;
Expand All @@ -40,7 +44,7 @@ class CredentialLeakDialogControllerTest : public testing::Test {
public:
void SetUpController(password_manager::CredentialLeakType leak_type) {
controller_ = std::make_unique<CredentialLeakDialogControllerImpl>(
&ui_controller_mock_, leak_type);
&ui_controller_mock_, leak_type, GURL(kUrl), kUsername);
}

base::HistogramTester& histogram_tester() { return histogram_tester_; }
Expand Down Expand Up @@ -159,7 +163,8 @@ TEST_F(CredentialLeakDialogControllerTest,
EXPECT_CALL(leak_prompt(), ShowCredentialLeakPrompt());
controller().ShowCredentialLeakPrompt(&leak_prompt());

EXPECT_CALL(ui_controller_mock(), StartAutomatedPasswordChange);
EXPECT_CALL(ui_controller_mock(), StartAutomatedPasswordChange(
GURL(kUrl), std::u16string(kUsername)));
EXPECT_CALL(ui_controller_mock(), OnLeakDialogHidden());
controller().OnAcceptDialog();

Expand Down
17 changes: 11 additions & 6 deletions chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "base/notreached.h"
#include "base/strings/utf_string_conversions.h"
#include "base/timer/timer.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/autofill_assistant/password_change/apc_client.h"
#include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/password_manager/account_password_store_factory.h"
#include "chrome/browser/password_manager/chrome_password_manager_client.h"
Expand Down Expand Up @@ -276,7 +278,8 @@ void ManagePasswordsUIController::OnPasswordAutofilled(

void ManagePasswordsUIController::OnCredentialLeak(
const password_manager::CredentialLeakType leak_type,
const GURL& origin) {
const GURL& url,
const std::u16string& username) {
// Existing dialog shouldn't be closed.
if (dialog_controller_)
return;
Expand All @@ -288,7 +291,7 @@ void ManagePasswordsUIController::OnCredentialLeak(
ClearPopUpFlagForBubble();

auto* raw_controller =
new CredentialLeakDialogControllerImpl(this, leak_type);
new CredentialLeakDialogControllerImpl(this, leak_type, url, username);
dialog_controller_.reset(raw_controller);
raw_controller->ShowCredentialLeakPrompt(
CreateCredentialLeakPrompt(raw_controller));
Expand Down Expand Up @@ -632,10 +635,12 @@ void ManagePasswordsUIController::NavigateToPasswordCheckup(
password_manager::LogPasswordCheckReferrer(referrer);
}

void ManagePasswordsUIController::StartAutomatedPasswordChange() {
// TODO(crbug.com/1321500): Implement once AutomatedPasswordCheckClient class
// exists.
NOTIMPLEMENTED();
void ManagePasswordsUIController::StartAutomatedPasswordChange(
const GURL& origin,
const std::u16string& username) {
ApcClient* apc_client = ApcClient::GetOrCreateForWebContents(web_contents());
// Start checks that no other run is ongoing, so we can always call it.
apc_client->Start(origin, base::UTF16ToUTF8(username), /*skip_login=*/true);
}

void ManagePasswordsUIController::EnableSync(const AccountInfo& account) {
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/ui/passwords/manage_passwords_ui_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ class ManagePasswordsUIController
const std::vector<const password_manager::PasswordForm*>*
federated_matches) override;
void OnCredentialLeak(password_manager::CredentialLeakType leak_dialog_type,
const GURL& origin) override;
const GURL& url,
const std::u16string& username) override;
void OnShowMoveToAccountBubble(
std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_move)
override;
Expand Down Expand Up @@ -230,7 +231,8 @@ class ManagePasswordsUIController
// PasswordsLeakDialogDelegate:
void NavigateToPasswordCheckup(
password_manager::PasswordCheckReferrer referrer) override;
void StartAutomatedPasswordChange() override;
void StartAutomatedPasswordChange(const GURL& origin,
const std::u16string& username) override;
void OnLeakDialogHidden() override;

enum class BubbleStatus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ MATCHER_P3(MatchesLoginAndURL,

// A random URL.
constexpr char kExampleUrl[] = "http://example.com";
constexpr char16_t kExampleUsername[] = u"Bob";

// Number of dismissals that for sure suppresses the bubble.
constexpr int kGreatDissmisalCount = 10;
Expand Down Expand Up @@ -1405,7 +1406,7 @@ TEST_F(ManagePasswordsUIControllerTest, SaveBubbleAfterLeakCheck) {
password_manager::IsSaved(false), password_manager::IsReused(false),
password_manager::IsSyncing(false),
password_manager::HasChangeScript(false)),
GURL(kExampleUrl));
GURL(kExampleUrl), kExampleUsername);
// The bubble is gone.
EXPECT_FALSE(controller()->opened_bubble());

Expand Down Expand Up @@ -1437,7 +1438,7 @@ TEST_F(ManagePasswordsUIControllerTest, UpdateBubbleAfterLeakCheck) {
password_manager::IsSaved(true), password_manager::IsReused(false),
password_manager::IsSyncing(false),
password_manager::HasChangeScript(false)),
GURL(kExampleUrl));
GURL(kExampleUrl), kExampleUsername);
// The bubble is gone.
EXPECT_FALSE(controller()->opened_bubble());

Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/passwords/passwords_client_ui_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ class PasswordsClientUIDelegate {
// Called when user credentials were leaked. This triggers the UI to prompt
// the user whether they would like to check their passwords.
virtual void OnCredentialLeak(password_manager::CredentialLeakType leak_type,
const GURL& origin) = 0;
const GURL& url,
const std::u16string& username) = 0;

// Called after a form was submitted. This triggers a bubble that allows to
// move the just used profile credential in |form| to the user's account.
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/ui/passwords/passwords_leak_dialog_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
#ifndef CHROME_BROWSER_UI_PASSWORDS_PASSWORDS_LEAK_DIALOG_DELEGATE_H_
#define CHROME_BROWSER_UI_PASSWORDS_PASSWORDS_LEAK_DIALOG_DELEGATE_H_

#include <string>

#include "components/password_manager/core/browser/ui/password_check_referrer.h"

class GURL;

// An interface for leak detection dialog implemented by
// ManagePasswordsUIController. Allows to retrieve the current state of the tab
// and notify about user actions.
Expand All @@ -20,7 +24,8 @@ class PasswordsLeakDialogDelegate {
password_manager::PasswordCheckReferrer referrer) = 0;

// Initiate an automated password change flow in the current tab.
virtual void StartAutomatedPasswordChange() = 0;
virtual void StartAutomatedPasswordChange(const GURL& origin,
const std::u16string& username) = 0;

protected:
virtual ~PasswordsLeakDialogDelegate() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
#ifndef CHROME_BROWSER_UI_PASSWORDS_PASSWORDS_LEAK_DIALOG_DELEGATE_MOCK_H_
#define CHROME_BROWSER_UI_PASSWORDS_PASSWORDS_LEAK_DIALOG_DELEGATE_MOCK_H_

#include <string>

#include "chrome/browser/ui/passwords/passwords_leak_dialog_delegate.h"
#include "testing/gmock/include/gmock/gmock.h"

class GURL;

class PasswordsLeakDialogDelegateMock : public PasswordsLeakDialogDelegate {
public:
PasswordsLeakDialogDelegateMock();
Expand All @@ -24,7 +28,10 @@ class PasswordsLeakDialogDelegateMock : public PasswordsLeakDialogDelegate {
NavigateToPasswordCheckup,
(password_manager::PasswordCheckReferrer),
(override));
MOCK_METHOD(void, StartAutomatedPasswordChange, (), (override));
MOCK_METHOD(void,
StartAutomatedPasswordChange,
(const GURL&, const std::u16string&),
(override));
};

#endif // CHROME_BROWSER_UI_PASSWORDS_PASSWORDS_LEAK_DIALOG_DELEGATE_MOCK_H_
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,6 @@ IN_PROC_BROWSER_TEST_F(PasswordBubbleInteractiveUiTest, LeakPromptHidesBubble) {

GetController()->OnCredentialLeak(
password_manager::CredentialLeakFlags::kPasswordSaved,
GURL("https://example.com"));
GURL("https://example.com"), std::u16string(u"Eve"));
EXPECT_FALSE(IsBubbleShowing());
}
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,8 @@ IN_PROC_BROWSER_TEST_F(PasswordDialogViewTest, PopupCredentialsLeakedPrompt) {
CredentialLeakType leak_type = CredentialLeakFlags::kPasswordSaved |
CredentialLeakFlags::kPasswordUsedOnOtherSites;
GURL origin("https://example.com");
controller()->OnCredentialLeak(leak_type, origin);
std::u16string username(u"Eve");
controller()->OnCredentialLeak(leak_type, origin, username);
ASSERT_TRUE(controller()->current_credential_leak_prompt());
EXPECT_EQ(password_manager::ui::INACTIVE_STATE, controller()->GetState());
CredentialLeakDialogView* dialog =
Expand Down Expand Up @@ -536,11 +537,12 @@ void PasswordDialogViewTest::ShowUi(const std::string& name) {
}

GURL origin("https://example.com");
std::u16string username(u"Eve");
if (name == "CredentialLeak") {
CredentialLeakType leak_type =
CredentialLeakFlags::kPasswordSaved |
CredentialLeakFlags::kPasswordUsedOnOtherSites;
controller()->OnCredentialLeak(leak_type, origin);
controller()->OnCredentialLeak(leak_type, origin, username);
return;
}

Expand Down

0 comments on commit 9dbe052

Please sign in to comment.