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

[iOS] Username editing error message

This change adds username input validation. If the new username is
already used for the same website username input will be highlighted and
Done button will be disabled.

Bug: 1137475
Change-Id: I1fa661b99f4d0f14805b13d0b4cf6ca0e8a2f16c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463235
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818507}
parent 8aad71d2
......@@ -1768,6 +1768,9 @@ Follow the steps below:
<message name="IDS_IOS_CONFIRM_PASSWORD_EDIT" desc="Confirm button inside confirmation alert when the user is trying to edit password [iOS only]" meaning="Save edited password">
Save Password
</message>
<message name="IDS_IOS_USERNAME_ALREADY_USED" desc="Error description telling the user that entered username is already used for this site [iOS only]">
You already saved this username for this site
</message>
<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>
......
9d3542f1ae57766f337355077cc49689482c2789
\ No newline at end of file
......@@ -65,6 +65,9 @@ class IOSChromePasswordCheckManager
std::vector<password_manager::CredentialWithPassword>
GetCompromisedCredentials() const;
password_manager::SavedPasswordsPresenter::SavedPasswordsView
GetAllCredentials() const;
password_manager::SavedPasswordsPresenter::SavedPasswordsView
GetSavedPasswordsFor(
const password_manager::CredentialWithPassword& credential) const;
......
......@@ -151,6 +151,11 @@ IOSChromePasswordCheckManager::GetCompromisedCredentials() const {
return insecure_credentials_manager_.GetCompromisedCredentials();
}
password_manager::SavedPasswordsPresenter::SavedPasswordsView
IOSChromePasswordCheckManager::GetAllCredentials() const {
return saved_passwords_presenter_.GetSavedPasswords();
}
password_manager::SavedPasswordsPresenter::SavedPasswordsView
IOSChromePasswordCheckManager::GetSavedPasswordsFor(
const CredentialWithPassword& credential) const {
......
......@@ -72,6 +72,7 @@ source_set("password_details_ui") {
"//ios/chrome/browser/ui/table_view/cells:cells_constants",
"//ios/chrome/browser/ui/util",
"//ios/chrome/common/ui/colors",
"//ios/chrome/common/ui/elements:popover_label_view_controller",
"//ios/chrome/common/ui/reauthentication",
"//ios/chrome/common/ui/util",
"//ui/base",
......
......@@ -28,6 +28,9 @@ using InsecureCredentialsView =
std::unique_ptr<PasswordCheckObserverBridge> _passwordCheckObserver;
}
// List of the usernames for the same domain.
@property(nonatomic, strong) NSSet<NSString*>* usernamesWithSameDomain;
@end
@implementation PasswordDetailsMediator
......@@ -40,6 +43,16 @@ using InsecureCredentialsView =
_password = passwordForm;
_passwordCheckObserver.reset(
new PasswordCheckObserverBridge(self, manager));
NSMutableSet<NSString*>* usernames = [[NSMutableSet alloc] init];
auto forms = manager->GetAllCredentials();
for (const auto& form : forms) {
if (form.signon_realm == passwordForm.signon_realm) {
[usernames addObject:base::SysUTF16ToNSString(form.username_value)];
}
}
[usernames
removeObject:base::SysUTF16ToNSString(passwordForm.username_value)];
_usernamesWithSameDomain = usernames;
}
return self;
}
......@@ -73,6 +86,12 @@ using InsecureCredentialsView =
[self fetchPasswordWith:_manager->GetCompromisedCredentials()];
}
- (BOOL)isUsernameReused:(NSString*)newUsername {
// It is more efficient to check set of the usernames for the same origin
// instead of delegating this to the |_manager|.
return [self.usernamesWithSameDomain containsObject:newUsername];
}
#pragma mark - PasswordCheckObserver
- (void)passwordCheckStateDidChange:(PasswordCheckState)state {
......
......@@ -23,11 +23,13 @@
#import "ios/chrome/browser/ui/settings/password/password_details/password_details_table_view_controller_delegate.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_edit_item.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_edit_item_delegate.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_item.h"
#include "ios/chrome/browser/ui/ui_feature_flags.h"
#include "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/common/ui/colors/UIColor+cr_semantic_colors.h"
#import "ios/chrome/common/ui/colors/semantic_color_names.h"
#import "ios/chrome/common/ui/elements/popover_label_view_controller.h"
#import "ios/chrome/common/ui/reauthentication/reauthentication_module.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
#include "ios/chrome/grit/ios_strings.h"
......@@ -68,7 +70,7 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
} // namespace
@interface PasswordDetailsTableViewController ()
@interface PasswordDetailsTableViewController () <TableViewTextEditItemDelegate>
// Password which is shown on the screen.
@property(nonatomic, strong) PasswordDetails* password;
......@@ -82,6 +84,10 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
// The text item related to the password value.
@property(nonatomic, strong) TableViewTextEditItem* passwordTextItem;
// The view used to anchor error alert which is shown for the username. This is
// image icon in the |usernameTextItem| cell.
@property(nonatomic, weak) UIView* usernameErrorAnchorView;
@end
@implementation PasswordDetailsTableViewController
......@@ -193,12 +199,15 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
item.textFieldName =
l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME);
item.textFieldValue = self.password.username;
if (base::FeatureList::IsEnabled(
// If password is missing (federated credential) don't allow to edit username.
if ([self.password.password length] &&
base::FeatureList::IsEnabled(
password_manager::features::kEditPasswordsInSettings)) {
item.textFieldEnabled = self.tableView.editing;
item.hideIcon = !self.tableView.editing;
item.autoCapitalizationType = UITextAutocapitalizationTypeNone;
item.returnKeyType = UIReturnKeyDone;
item.delegate = self;
} else {
item.textFieldEnabled = NO;
item.hideIcon = YES;
......@@ -219,6 +228,7 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
item.autoCapitalizationType = UITextAutocapitalizationTypeNone;
item.keyboardType = UIKeyboardTypeURL;
item.returnKeyType = UIReturnKeyDone;
item.delegate = self;
// During editing password is exposed so eye icon shouldn't be shown.
if (!self.tableView.editing) {
......@@ -357,6 +367,11 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
TableViewTextEditCell* textFieldCell =
base::mac::ObjCCastStrict<TableViewTextEditCell>(cell);
textFieldCell.textField.delegate = self;
[textFieldCell.identifyingIconButton
addTarget:self
action:@selector(didTapUsernameErrorInfo:)
forControlEvents:UIControlEventTouchUpInside];
self.usernameErrorAnchorView = textFieldCell.iconView;
break;
}
case ItemTypePassword: {
......@@ -401,7 +416,35 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
[self reloadData];
}
#pragma mark - Private
#pragma mark - TableViewTextEditItemDelegate
- (void)tableViewItemDidBeginEditing:(TableViewTextEditItem*)tableViewItem {
[self reconfigureCellsForItems:@[
self.usernameTextItem, self.passwordTextItem
]];
}
- (void)tableViewItemDidChange:(TableViewTextEditItem*)tableViewItem {
BOOL isInputValid = YES;
if (tableViewItem == self.usernameTextItem) {
isInputValid = [self checkIfValidUsername];
}
self.navigationItem.rightBarButtonItem.enabled = isInputValid;
}
- (void)tableViewItemDidEndEditing:(TableViewTextEditItem*)tableViewItem {
// Check if the item is equal to the current username or password item as when
// editing finished reloadData is called.
if (tableViewItem == self.usernameTextItem) {
[self reconfigureCellsForItems:@[ self.usernameTextItem ]];
} else if (tableViewItem == self.passwordTextItem) {
[self reconfigureCellsForItems:@[ self.passwordTextItem ]];
}
}
#pragma mark - SettingsRootTableViewController
// Called when user tapped Delete button during editing. It means presented
// password should be deleted.
......@@ -419,6 +462,8 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
return !self.editing;
}
#pragma mark - Private
// Applies tint colour and resizes image.
- (UIImage*)getCompromisedIcon {
UIImage* image = [UIImage imageNamed:@"settings_unsafe_state"];
......@@ -562,6 +607,22 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
}
}
// Checks if the username is valid and updates item accordingly.
- (BOOL)checkIfValidUsername {
DCHECK(self.password.username);
NSString* newUsernameValue = self.usernameTextItem.textFieldValue;
BOOL usernameChanged =
![newUsernameValue isEqualToString:self.password.username];
BOOL showUsernameAlreadyUsed =
usernameChanged && [self.delegate isUsernameReused:newUsernameValue];
self.usernameTextItem.hasValidText = !showUsernameAlreadyUsed;
self.usernameTextItem.identifyingIconEnabled = showUsernameAlreadyUsed;
[self reconfigureCellsForItems:@[ self.usernameTextItem ]];
return !showUsernameAlreadyUsed;
}
#pragma mark - Actions
// Called when the user tapped on the show/hide button near password.
......@@ -578,6 +639,33 @@ typedef NS_ENUM(NSInteger, ReauthenticationReason) {
}
}
// Called when the user tap error info icon in the username input.
- (void)didTapUsernameErrorInfo:(UIButton*)buttonView {
NSString* text = l10n_util::GetNSString(IDS_IOS_USERNAME_ALREADY_USED);
NSAttributedString* attributedText = [[NSAttributedString alloc]
initWithString:text
attributes:@{
NSForegroundColorAttributeName :
[UIColor colorNamed:kTextSecondaryColor],
NSFontAttributeName :
[UIFont preferredFontForTextStyle:UIFontTextStyleSubheadline]
}];
PopoverLabelViewController* errorInfoPopover =
[[PopoverLabelViewController alloc]
initWithPrimaryAttributedString:attributedText
secondaryAttributedString:nil];
errorInfoPopover.popoverPresentationController.sourceView =
self.usernameErrorAnchorView;
errorInfoPopover.popoverPresentationController.sourceRect =
self.usernameErrorAnchorView.bounds;
errorInfoPopover.popoverPresentationController.permittedArrowDirections =
UIPopoverArrowDirectionAny;
[self presentViewController:errorInfoPopover animated:YES completion:nil];
}
// Returns an array of UIMenuItems to display in a context menu on the site
// cell.
- (NSArray*)getMenuItemsFor:(NSInteger)itemType {
......
......@@ -15,6 +15,9 @@
(PasswordDetailsTableViewController*)viewController
didEditPasswordDetails:(PasswordDetails*)password;
// Checks if the username is reused for the same domain.
- (BOOL)isUsernameReused:(NSString*)newUsername;
@end
#endif // IOS_CHROME_BROWSER_UI_SETTINGS_PASSWORD_PASSWORD_DETAILS_PASSWORD_DETAILS_TABLE_VIEW_CONTROLLER_DELEGATE_H_
......@@ -89,6 +89,10 @@ constexpr char kPassword[] = "test";
self.password = password;
}
- (BOOL)isUsernameReused:(NSString*)newUsername {
return NO;
}
@end
@interface FakeSnackbarImplementation : NSObject <SnackbarCommands>
......
......@@ -53,6 +53,7 @@ using chrome_test_util::NavigationBarDoneButton;
using chrome_test_util::SettingsDoneButton;
using chrome_test_util::SettingsMenuBackButton;
using chrome_test_util::TurnSettingsSwitchOn;
using chrome_test_util::TextFieldForCellWithLabelId;
namespace {
......@@ -135,23 +136,17 @@ GREYLayoutConstraint* Below() {
// Matcher for the Copy site button in Password Details view.
id<GREYMatcher> PasswordDetailWebsite() {
return grey_allOf(
grey_accessibilityID(GetTextFieldForID(IDS_IOS_SHOW_PASSWORD_VIEW_SITE)),
grey_kindOfClassName(@"UITextField"), nil);
return TextFieldForCellWithLabelId(IDS_IOS_SHOW_PASSWORD_VIEW_SITE);
}
// Matcher for the Copy username button in Password Details view.
id<GREYMatcher> PasswordDetailUsername() {
return grey_allOf(grey_accessibilityID(
GetTextFieldForID(IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME)),
grey_kindOfClassName(@"UITextField"), nil);
return TextFieldForCellWithLabelId(IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME);
}
// Matcher for the Copy password button in Password Details view.
id<GREYMatcher> PasswordDetailPassword() {
return grey_allOf(grey_accessibilityID(
GetTextFieldForID(IDS_IOS_SHOW_PASSWORD_VIEW_PASSWORD)),
grey_kindOfClassName(@"UITextField"), nil);
return TextFieldForCellWithLabelId(IDS_IOS_SHOW_PASSWORD_VIEW_PASSWORD);
}
// Matcher for the Show password button in Password Details view.
......@@ -316,7 +311,8 @@ void CopyPasswordDetailWithID(int detail_id) {
// TODO(crbug.com/1075494): Remove this once feature is launched.
config.features_enabled.push_back(password_manager::features::kPasswordCheck);
if ([self isRunningTest:@selector(testEditUsername)]) {
if ([self isRunningTest:@selector(testEditUsername)] ||
[self isRunningTest:@selector(testEditUsernameFails)]) {
config.features_enabled.push_back(
password_manager::features::kEditPasswordsInSettings);
}
......@@ -1486,4 +1482,53 @@ void CopyPasswordDetailWithID(int detail_id) {
performAction:grey_tap()];
}
// Checks that attempts to edit a username to a value which is already used for
// the same domain fails.
- (void)testEditUsernameFails {
GREYAssert(
[PasswordSettingsAppInterface saveExamplePassword:@"concrete password"
userName:@"concrete username1"
origin:@"https://example.com"],
@"Stored form was not found in the PasswordStore results.");
GREYAssert(
[PasswordSettingsAppInterface saveExamplePassword:@"concrete password"
userName:@"concrete username2"
origin:@"https://example.com"],
@"Stored form was not found in the PasswordStore results.");
OpenPasswordSettings();
[GetInteractionForPasswordEntry(@"example.com, concrete username1")
performAction:grey_tap()];
// Check the snackbar in case of successful reauthentication.
[PasswordSettingsAppInterface setUpMockReauthenticationModule];
[PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
ReauthenticationResult::kSuccess];
TapEdit();
[[EarlGrey selectElementWithMatcher:PasswordDetailUsername()]
assertWithMatcher:grey_textFieldValue(@"concrete username1")];
[[EarlGrey selectElementWithMatcher:PasswordDetailUsername()]
performAction:grey_clearText()];
[[EarlGrey selectElementWithMatcher:PasswordDetailUsername()]
performAction:grey_typeText(@"concrete username2")];
[[EarlGrey selectElementWithMatcher:NavigationBarDoneButton()]
assertWithMatcher:grey_allOf(grey_sufficientlyVisible(),
grey_not(grey_enabled()), nil)];
[[EarlGrey selectElementWithMatcher:SettingsMenuBackButton()]
performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher:SettingsMenuBackButton()]
performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher:SettingsDoneButton()]
performAction:grey_tap()];
}
@end
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