Commit f669bad9 authored by Viktor Semeniuk's avatar Viktor Semeniuk Committed by Commit Bot

[iOS] Warning before using a password from a different origin

Before allowing users to use another password, we should warn users that
they should trust the current website because of the security risk

Bug: 1137459
Change-Id: Id4d003ea1d747862041ce6a521f0f884f63df3cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476154
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: default avatarJavier Ernesto Flores Robles <javierrobles@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821399}
parent 997fa51d
......@@ -1774,6 +1774,17 @@ Follow the steps below:
<message name="IDS_IOS_CANCEL_PASSWORD_EDIT" desc="Cancel button inside confirmation alert when the user is trying to edit password [iOS only]" meaning="Cancels editing">
Cancel
</message>
<message name="IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_TITLE" desc="Title for the confirmation dialog which is shown when user wants to use other passwords. [iOS only]">
Use other password
</message>
<message name="IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_DESCRIPTION" desc="Description for the confirmation dialog which is shown when user wants to use other passwords. [iOS only]">
If you trust <ph name="TIME">$1<ex>example.com</ex></ph>, you can use a saved password from another site.
Try to use a unique password for every site.
</message>
<message name="IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_CONTINUE" desc="Action button for confirmation dialog which is shown when user wants to use other passwords. [iOS only]" meaning="User acknowledge warning and wants to proceed.">
Continue
</message>
<message name="IDS_IOS_LAST_COMPLETED_CHECK" desc="Footer for Password Check section which shows the timestamp of the last check." meaning="Time when passwords were checked last time.">
Last checked <ph name="TIME">$1<ex>10 minutes ago</ex></ph>.
</message>
......
6744fea42c080e6601ed3753cf143707eb381514
\ No newline at end of file
6744fea42c080e6601ed3753cf143707eb381514
\ No newline at end of file
6744fea42c080e6601ed3753cf143707eb381514
\ No newline at end of file
......@@ -31,6 +31,7 @@ source_set("form_input_accessory") {
"//ios/chrome/browser/main",
"//ios/chrome/browser/passwords",
"//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/alert_coordinator",
"//ios/chrome/browser/ui/autofill/manual_fill",
"//ios/chrome/browser/ui/autofill/manual_fill:manual_fill_ui",
"//ios/chrome/browser/ui/commands",
......
......@@ -8,15 +8,18 @@
#include "base/ios/ios_util.h"
#include "base/mac/foundation_util.h"
#include "base/strings/utf_string_conversions.h"
#include "components/autofill/core/browser/personal_data_manager.h"
#include "components/autofill/core/common/autofill_features.h"
#import "components/autofill/ios/browser/js_suggestion_manager.h"
#include "components/keyed_service/core/service_access_type.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/autofill/personal_data_manager_factory.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/main/browser.h"
#include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
#import "ios/chrome/browser/ui/alert_coordinator/alert_coordinator.h"
#import "ios/chrome/browser/ui/autofill/form_input_accessory/form_input_accessory_mediator.h"
#import "ios/chrome/browser/ui/autofill/form_input_accessory/form_input_accessory_view_controller.h"
#import "ios/chrome/browser/ui/autofill/manual_fill/address_coordinator.h"
......@@ -70,6 +73,9 @@
// Reauthentication Module used for re-authentication.
@property(nonatomic, strong) ReauthenticationModule* reauthenticationModule;
// Modal alert.
@property(nonatomic, strong) AlertCoordinator* alertCoordinator;
@end
@implementation FormInputAccessoryCoordinator
......@@ -263,12 +269,7 @@
}
- (void)openAllPasswordsPicker {
[self reset];
self.allPasswordCoordinator = [[ManualFillAllPasswordCoordinator alloc]
initWithBaseViewController:self.baseViewController
browser:self.browser
injectionHandler:self.injectionHandler];
[self.allPasswordCoordinator start];
[self showConfirmationDialogToUseOtherPassword];
}
#pragma mark - CardCoordinatorDelegate
......@@ -344,4 +345,50 @@
completion:nil];
}
#pragma mark - Private
// Shows confirmation dialog before opening Other passwords.
- (void)showConfirmationDialogToUseOtherPassword {
WebStateList* webStateList = self.browser->GetWebStateList();
const GURL& URL = webStateList->GetActiveWebState()->GetLastCommittedURL();
base::string16 origin = base::ASCIIToUTF16(
password_manager::GetShownOrigin(url::Origin::Create(URL)));
NSString* title =
l10n_util::GetNSString(IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_TITLE);
NSString* message = l10n_util::GetNSStringF(
IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_DESCRIPTION, origin);
self.alertCoordinator = [[AlertCoordinator alloc]
initWithBaseViewController:self.baseViewController
browser:self.browser
title:title
message:message];
__weak __typeof__(self) weakSelf = self;
[self.alertCoordinator addItemWithTitle:l10n_util::GetNSString(IDS_CANCEL)
action:nil
style:UIAlertActionStyleCancel];
NSString* actionTitle =
l10n_util::GetNSString(IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_CONTINUE);
[self.alertCoordinator addItemWithTitle:actionTitle
action:^{
[weakSelf showAllPasswords];
}
style:UIAlertActionStyleDefault];
[self.alertCoordinator start];
}
// Opens other passwords.
- (void)showAllPasswords {
[self reset];
self.allPasswordCoordinator = [[ManualFillAllPasswordCoordinator alloc]
initWithBaseViewController:self.baseViewController
browser:self.browser
injectionHandler:self.injectionHandler];
[self.allPasswordCoordinator start];
}
@end
......@@ -230,6 +230,8 @@ source_set("eg2_tests") {
"//base",
"//base/test:test_support",
"//components/autofill/core/browser:test_support",
"//components/password_manager/core/browser",
"//components/strings",
"//ios/chrome/app/strings:ios_strings_grit",
"//ios/chrome/browser/ui/autofill:eg_test_support+eg2",
"//ios/chrome/browser/ui/settings/autofill:feature_flags",
......@@ -241,6 +243,7 @@ source_set("eg2_tests") {
"//ios/third_party/earl_grey2:test_lib",
"//ios/web/public/test:element_selector",
"//net:test_support",
"//ui/base",
"//url",
]
frameworks = [ "UIKit.framework" ]
......
......@@ -4,7 +4,10 @@
#include "base/ios/ios_util.h"
#include "base/strings/sys_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
#import "base/test/ios/wait_util.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/ui/autofill/autofill_app_interface.h"
#include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/earl_grey/chrome_actions.h"
......@@ -14,12 +17,14 @@
#import "ios/testing/earl_grey/earl_grey_test.h"
#include "ios/web/public/test/element_selector.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "ui/base/l10n/l10n_util_mac.h"
#include "url/gurl.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
using chrome_test_util::ButtonWithAccessibilityLabelId;
using chrome_test_util::CancelButton;
using chrome_test_util::ManualFallbackKeyboardIconMatcher;
using chrome_test_util::ManualFallbackPasswordIconMatcher;
......@@ -59,6 +64,19 @@ id<GREYMatcher> NotSecureWebsiteAlert() {
IDS_IOS_MANUAL_FALLBACK_NOT_SECURE_TITLE);
}
// Matcher for the confirmation dialog Continue button.
id<GREYMatcher> ConfirmUsingOtherPasswordButton() {
return grey_allOf(ButtonWithAccessibilityLabelId(
IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_CONTINUE),
grey_interactable(), nullptr);
}
// Matcher for the confirmation dialog Cancel button.
id<GREYMatcher> CancelUsingOtherPasswordButton() {
return grey_allOf(ButtonWithAccessibilityLabelId(IDS_CANCEL),
grey_interactable(), nullptr);
}
// Polls the JavaScript query |java_script_condition| until the returned
// |boolValue| is YES with a kWaitForActionTimeout timeout.
BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
......@@ -78,6 +96,10 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
// Integration Tests for Mannual Fallback Passwords View Controller.
@interface PasswordViewControllerTestCase : ChromeTestCase
// URL of the current page.
@property(assign) GURL URL;
@end
@implementation PasswordViewControllerTestCase
......@@ -85,8 +107,8 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
- (void)setUp {
[super setUp];
GREYAssertTrue(self.testServer->Start(), @"Test server failed to start.");
const GURL URL = self.testServer->GetURL(kFormHTMLFile);
[ChromeEarlGrey loadURL:URL];
self.URL = self.testServer->GetURL(kFormHTMLFile);
[ChromeEarlGrey loadURL:self.URL];
[ChromeEarlGrey waitForWebStateContainingText:"hello!"];
[AutofillAppInterface saveExamplePasswordForm];
}
......@@ -200,12 +222,47 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
[[EarlGrey selectElementWithMatcher:ManualFallbackOtherPasswordsMatcher()]
performAction:grey_tap()];
base::string16 origin = base::ASCIIToUTF16(
password_manager::GetShownOrigin(url::Origin::Create(self.URL)));
NSString* title = l10n_util::GetNSStringF(
IDS_IOS_CONFIRM_USING_OTHER_PASSWORD_DESCRIPTION, origin);
[[EarlGrey selectElementWithMatcher:grey_text(title)]
assertWithMatcher:grey_notNil()];
// Acknowledge concerns using other passwords on a website.
[[EarlGrey selectElementWithMatcher:ConfirmUsingOtherPasswordButton()]
performAction:grey_tap()];
// Verify the use other passwords opened.
[[EarlGrey
selectElementWithMatcher:ManualFallbackOtherPasswordsDismissMatcher()]
assertWithMatcher:grey_sufficientlyVisible()];
}
// Tests that the "Use Other Password..." screen won't open if canceled.
- (void)testUseOtherPasswordActionCloses {
// Bring up the keyboard.
[[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
performAction:TapWebElementWithId(kFormElementUsername)];
// Tap on the passwords icon.
[[EarlGrey selectElementWithMatcher:ManualFallbackPasswordIconMatcher()]
performAction:grey_tap()];
// Tap the "Manage Passwords..." action.
[[EarlGrey selectElementWithMatcher:ManualFallbackOtherPasswordsMatcher()]
performAction:grey_tap()];
// Cancel using other passwords on a website.
[[EarlGrey selectElementWithMatcher:CancelUsingOtherPasswordButton()]
performAction:grey_tap()];
// Verify the use other passwords not opened.
[[EarlGrey
selectElementWithMatcher:ManualFallbackOtherPasswordsDismissMatcher()]
assertWithMatcher:grey_nil()];
}
// Tests that returning from "Use Other Password..." leaves the view and icons
// in the right state.
- (void)testPasswordsStateAfterPresentingUseOtherPassword {
......@@ -225,6 +282,10 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
[[EarlGrey selectElementWithMatcher:ManualFallbackOtherPasswordsMatcher()]
performAction:grey_tap()];
// Acknowledge concerns using other passwords on a website.
[[EarlGrey selectElementWithMatcher:ConfirmUsingOtherPasswordButton()]
performAction:grey_tap()];
// Verify the use other passwords opened.
[[EarlGrey
selectElementWithMatcher:ManualFallbackOtherPasswordsDismissMatcher()]
......
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