Skip to content

Commit

Permalink
[IBAN validation] Add IBAN validation to add IBAN dialog.
Browse files Browse the repository at this point in the history
This CL does the followings:
1. IBAN validation to add IBAN add and edit dialog on Chrome payments
settings page, so that when in the "Add" mode, the IBAN value is
empty, the "save" button is grey. In "Edit" mode, the value has been
populated, the button is blue.
2. Remove validation check in payments_section_iban_test.is as the
invalid cases are fully covered at:
https://source.chromium.org/chromium/chromium/src/+/main:components/autofill/core/browser/data_model/iban_unittest.cc;drc=b80ba3c4f28e09e826c6def5ceb0f39c74393f7e;l=158-230

Bug: 1349109
Change-Id: Ic88e9fcc4fd2156cc5d64de5805dcee79816b6bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4318350
Reviewed-by: Vinny Persky <vinnypersky@google.com>
Reviewed-by: Jared Saul <jsaul@google.com>
Commit-Queue: Qihui Zhao <qihuizhao@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: John Lee <johntlee@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1117604}
  • Loading branch information
Qihui Zhao authored and Chromium LUCI CQ committed Mar 15, 2023
1 parent c947fef commit 38e9918
Show file tree
Hide file tree
Showing 15 changed files with 129 additions and 34 deletions.
Expand Up @@ -23,6 +23,7 @@
#include "components/autofill/core/browser/browser_autofill_manager.h"
#include "components/autofill/core/browser/data_model/autofill_profile.h"
#include "components/autofill/core/browser/data_model/credit_card.h"
#include "components/autofill/core/browser/data_model/iban.h"
#include "components/autofill/core/browser/form_data_importer.h"
#include "components/autofill/core/browser/payments/local_card_migration_manager.h"
#include "components/autofill/core/browser/payments/virtual_card_enrollment_flow.h"
Expand Down Expand Up @@ -715,6 +716,17 @@ ExtensionFunction::ResponseAction AutofillPrivateGetIbanListFunction::Run() {
api::autofill_private::GetIbanList::Results::Create(iban_list)));
}

////////////////////////////////////////////////////////////////////////////////
// AutofillPrivateIsValidIbanFunction

ExtensionFunction::ResponseAction AutofillPrivateIsValidIbanFunction::Run() {
absl::optional<api::autofill_private::IsValidIban::Params> parameters =
api::autofill_private::IsValidIban::Params::Create(args());
EXTENSION_FUNCTION_VALIDATE(parameters);
return RespondNow(WithArguments(
autofill::IBAN::IsValid(base::UTF8ToUTF16(parameters->iban_value))));
}

////////////////////////////////////////////////////////////////////////////////
// AutofillPrivateGetUpiIdListFunction

Expand Down
Expand Up @@ -268,6 +268,23 @@ class AutofillPrivateGetIbanListFunction : public ExtensionFunction {
ResponseAction Run() override;
};

class AutofillPrivateIsValidIbanFunction : public ExtensionFunction {
public:
AutofillPrivateIsValidIbanFunction() = default;
AutofillPrivateIsValidIbanFunction(
const AutofillPrivateIsValidIbanFunction&) = delete;
AutofillPrivateIsValidIbanFunction& operator=(
const AutofillPrivateIsValidIbanFunction&) = delete;
DECLARE_EXTENSION_FUNCTION("autofillPrivate.isValidIban",
AUTOFILLPRIVATE_ISVALIDIBAN)

protected:
~AutofillPrivateIsValidIbanFunction() override = default;

// ExtensionFunction overrides.
ResponseAction Run() override;
};

class AutofillPrivateGetUpiIdListFunction : public ExtensionFunction {
public:
AutofillPrivateGetUpiIdListFunction() = default;
Expand Down
Expand Up @@ -130,4 +130,9 @@ IN_PROC_BROWSER_TEST_P(AutofillPrivateApiTest, removeExistingIban) {
EXPECT_EQ(1, user_action_tester.GetActionCount("AutofillIbanDeleted"));
}

IN_PROC_BROWSER_TEST_P(AutofillPrivateApiTest, isValidIban) {
base::UserActionTester user_action_tester;
EXPECT_TRUE(RunAutofillSubtest("isValidIban")) << message_;
}

} // namespace extensions
Expand Up @@ -35,6 +35,7 @@
<div slot="title">[[title_]]</div>
<div slot="body">
<cr-input id="valueInput" label="$i18n{addPaymentMethodIban}"
on-input="updateSaveIbanButtonEnablement_"
value="{{value_}}" autofocus>
</cr-input>
<cr-input id="nicknameInput" label="$i18n{ibanNickname}"
Expand All @@ -51,8 +52,7 @@
<cr-button id="cancelButton" class="cancel-button"
on-click="onCancelButtonClick_">$i18n{cancel}</cr-button>
<cr-button id="saveButton" class="action-button"
on-click="onIbanSaveButtonClick_"
disabled="[[!saveIbanEnabled_(value_)]]">
on-click="onIbanSaveButtonClick_">
$i18n{save}
</cr-button>
</div>
Expand Down
26 changes: 16 additions & 10 deletions chrome/browser/resources/settings/autofill_page/iban_edit_dialog.ts
Expand Up @@ -23,12 +23,7 @@ import {I18nMixin} from 'chrome://resources/cr_elements/i18n_mixin.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {getTemplate} from './iban_edit_dialog.html.js';

/**
* Regular expression for valid IBAN value.
*/
const IBAN_VALID_REGEX: RegExp = new RegExp(
'^[a-zA-Z]{2}[0-9]{2}[a-zA-Z0-9]{4}[0-9]{7}([a-zA-Z0-9]?){0,16}$');
import {PaymentsManagerImpl, PaymentsManagerProxy} from './payments_manager_proxy.js';

declare global {
interface HTMLElementEventMap {
Expand Down Expand Up @@ -85,6 +80,8 @@ export class SettingsIbanEditDialogElement extends
private value_?: string;
private nickname_?: string;
private title_: string;
private paymentsManager_: PaymentsManagerProxy =
PaymentsManagerImpl.getInstance();

override connectedCallback() {
super.connectedCallback();
Expand All @@ -94,6 +91,7 @@ export class SettingsIbanEditDialogElement extends
this.value_ = this.iban.value;
this.nickname_ = this.iban.nickname;
}
this.$.saveButton.disabled = true;
this.$.dialog.showModal();
}

Expand Down Expand Up @@ -123,16 +121,24 @@ export class SettingsIbanEditDialogElement extends
this.close();
}

private saveIbanEnabled_(): boolean {
private updateSaveIbanButtonEnablement_() {
this.isValidIban().then(isValid => {
this.$.saveButton.disabled = !isValid;
});
}

private async isValidIban(): Promise<boolean> {
if (!this.value_) {
return false;
return Promise.resolve(false);
}
// The save button is enabled if the value of the IBAN is invalid (after
// removing all whitespace from it).
const ibanWithoutWhitespace = this.value_.replace(/\s/g, '');
return !!IBAN_VALID_REGEX.test(ibanWithoutWhitespace!);
const isValid = await this.paymentsManager_.isValidIban(
this.value_!.replace(/\s/g, ''));
return isValid;
}


/**
* @param nickname of the IBAN, undefined when not set.
* @return nickname character length.
Expand Down
Expand Up @@ -29,6 +29,9 @@ export interface PaymentsManagerProxy {
*/
getIbanList(): Promise<chrome.autofillPrivate.IbanEntry[]>;

/** @param ibanValue Returns true if the given ibanValue is valid. */
isValidIban(ibanValue: string): Promise<boolean>;

/** @param guid The GUID of the credit card to remove. */
removeCreditCard(guid: string): void;

Expand Down Expand Up @@ -106,6 +109,10 @@ export class PaymentsManagerImpl implements PaymentsManagerProxy {
return chrome.autofillPrivate.getIbanList();
}

isValidIban(ibanValue: string) {
return chrome.autofillPrivate.isValidIban(ibanValue);
}

removeCreditCard(guid: string) {
chrome.autofillPrivate.removeEntry(guid);
}
Expand Down
7 changes: 7 additions & 0 deletions chrome/common/extensions/api/autofill_private.idl
Expand Up @@ -245,6 +245,7 @@ namespace autofillPrivate {
callback GetCreditCardListCallback = void(CreditCardEntry[] entries);
callback GetIbanListCallback = void(IbanEntry[] entries);
callback GetUpiIdListCallback = void(DOMString[] entries);
callback IsValidIbanCallback = void(boolean isValid);

interface Functions {
// Gets currently signed-in user profile info, no value is returned if
Expand Down Expand Up @@ -310,6 +311,12 @@ namespace autofillPrivate {
[supportsPromises] static void getIbanList(
GetIbanListCallback callback);

// Returns true if the given `ibanValue` is a valid IBAN.
// `callback`: Callback which will be called with the result of IBAN
// validation.
[supportsPromises] static void isValidIban(
DOMString ibanValue, IsValidIbanCallback callback);

// Clears the data associated with a wallet card which was saved
// locally so that the saved copy is masked (e.g., "Card ending
// in 1234").
Expand Down
32 changes: 25 additions & 7 deletions chrome/test/data/extensions/api_test/autofill_private/test.js
Expand Up @@ -22,6 +22,7 @@ var NUMBER = '4111 1111 1111 1111';
var EXP_MONTH = '02';
var EXP_YEAR = '2999';
var IBAN_VALUE = 'AD1400080001001234567890';
var INVALID_IBAN_VALUE = 'AD14000800010012345678900';

var failOnceCalled = function() {
chrome.test.fail();
Expand Down Expand Up @@ -524,6 +525,22 @@ var availableTests = [
countryCode: COUNTRY_CODE
}, handler1);
},

function isValidIban() {
var handler1 = function(isValidIban) {
// IBAN_VALUE should be valid, then follow up with invalid value.
chrome.test.assertTrue(isValidIban);
chrome.autofillPrivate.isValidIban(INVALID_IBAN_VALUE, handler2);
}

var handler2 = function(isValidIban) {
// INVALID_IBAN_VALUE is not valid.
chrome.test.assertFalse(isValidIban);
chrome.test.succeed();
}

chrome.autofillPrivate.isValidIban(IBAN_VALUE, handler1);
},
];

/** @const */
Expand All @@ -535,13 +552,14 @@ var TESTS_FOR_CONFIG = {
],
'addNewIbanNoNickname': ['addNewIbanNoNickname'],
'addNewIbanWithNickname': ['addNewIbanWithNickname'],
'noChangesToExistingIban': ['addNewIbanNoNickname','noChangesToExistingIban'],
'updateExistingIbanNoNickname': [
'addNewIbanNoNickname', 'updateExistingIban_NoNickname'],
'updateExistingIbanWithNickname': [
'addNewIbanNoNickname', 'updateExistingIban_WithNickname'],
'removeExistingIban': [
'addNewIbanNoNickname', 'removeExistingIban'],
'noChangesToExistingIban':
['addNewIbanNoNickname', 'noChangesToExistingIban'],
'updateExistingIbanNoNickname':
['addNewIbanNoNickname', 'updateExistingIban_NoNickname'],
'updateExistingIbanWithNickname':
['addNewIbanNoNickname', 'updateExistingIban_WithNickname'],
'removeExistingIban': ['addNewIbanNoNickname', 'removeExistingIban'],
'isValidIban': ['isValidIban'],
};

chrome.test.getConfig(function(config) {
Expand Down
Expand Up @@ -506,6 +506,7 @@ export class PaymentsManagerExpectations {
addedVirtualCards: number = 0;
requestedIbans: number = 0;
removedIbans: number = 0;
isValidIban: number = 0;
}

/**
Expand Down Expand Up @@ -535,6 +536,7 @@ export class TestPaymentsManager extends TestBrowserProxy implements
'removeCreditCard',
'removeIban',
'addVirtualCard',
'isValidIban',
]);

// Set these to have non-empty data.
Expand Down Expand Up @@ -602,6 +604,11 @@ export class TestPaymentsManager extends TestBrowserProxy implements
return Promise.resolve(this.data.ibans);
}

isValidIban(_ibanValue: string) {
this.methodCalled('isValidIban');
return Promise.resolve(true);
}


setIsUserVerifyingPlatformAuthenticatorAvailable(available: boolean|null) {
this.isUserVerifyingPlatformAuthenticatorAvailable_ = available;
Expand Down
40 changes: 25 additions & 15 deletions chrome/test/data/webui/settings/payments_section_iban_test.ts
Expand Up @@ -4,7 +4,7 @@

// clang-format off
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {PaymentsManagerImpl, SettingsIbanEditDialogElement} from 'chrome://settings/lazy_load.js';
import {CrInputElement, PaymentsManagerImpl, SettingsIbanEditDialogElement} from 'chrome://settings/lazy_load.js';
import {CrButtonElement, loadTimeData} from 'chrome://settings/settings.js';
import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {eventToPromise, whenAttributeIs} from 'chrome://webui-test/test_util.js';
Expand All @@ -14,6 +14,16 @@ import {createPaymentsSection, getDefaultExpectations} from './payments_section_

// clang-format on

/**
* Helper function to update IBAN value in the IBAN field.
*/
function updateIbanTextboxValue(valueInput: CrInputElement, value: string) {
valueInput.value = value;
valueInput.dispatchEvent(
new CustomEvent('input', {bubbles: true, composed: true}));
}


suite('PaymentsSectionIban', function() {
setup(function() {
document.body.innerHTML = window.trustedTypes!.emptyHTML;
Expand All @@ -26,14 +36,15 @@ suite('PaymentsSectionIban', function() {
});

/**
* Creates the Edit IBAN dialog.
* Creates the Add or Edit IBAN dialog.
*/
function createIbanDialog(ibanItem: chrome.autofillPrivate.IbanEntry):
SettingsIbanEditDialogElement {
const dialog = document.createElement('settings-iban-edit-dialog');
dialog.iban = ibanItem;
document.body.appendChild(dialog);
flush();
dialog.$.saveButton.disabled = false;
return dialog;
}

Expand Down Expand Up @@ -135,29 +146,28 @@ suite('PaymentsSectionIban', function() {

const saveButton = ibanDialog.$.saveButton;
assertTrue(!!saveButton);
// Can't be saved, because there's no value.
assertTrue(saveButton.disabled);

// Add invalid IBAN value.
// Add a valid IBAN value.
const valueInput = ibanDialog.$.valueInput;
valueInput.value = '11112222333344445678';
flush();
// Can't be saved, because the value of IBAN is invalid.
assertTrue(saveButton.disabled);
updateIbanTextboxValue(valueInput, 'FI1410093000123458');

// Add valid IBAN value.
valueInput.value = 'FI1410093000123458';
flush();
// Can be saved, because the value of IBAN is valid.
assertFalse(saveButton.disabled);
// Type in another valid IBAN value.
updateIbanTextboxValue(valueInput, 'IT60X0542811101000000123456');

const savePromise = eventToPromise('save-iban', ibanDialog);
saveButton.click();
const event = await savePromise;

assertEquals(undefined, event.detail.guid);
assertEquals('FI1410093000123458', event.detail.value);
assertEquals('IT60X0542811101000000123456', event.detail.value);
assertEquals('', event.detail.nickname);

const paymentsManager =
PaymentsManagerImpl.getInstance() as TestPaymentsManager;
const expectations = getDefaultExpectations();
expectations.isValidIban = 2;
expectations.listeningCreditCards = 0;
paymentsManager.assertExpectations(expectations);
});

test('verifyIbanEntryIsNotEditedAfterCancel', async function() {
Expand Down
Expand Up @@ -131,6 +131,7 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
const ibanDialog =
section.shadowRoot!.querySelector('settings-iban-edit-dialog');
assertTrue(!!ibanDialog);
ibanDialog.$.saveButton.disabled = false;
return ibanDialog!;
}

Expand Down Expand Up @@ -191,6 +192,7 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
const ibanDialog =
section.shadowRoot!.querySelector('settings-iban-edit-dialog');
assertTrue(!!ibanDialog);
ibanDialog.$.saveButton.disabled = false;
return ibanDialog;
}

Expand Down
1 change: 1 addition & 0 deletions chrome/test/data/webui/settings/payments_section_utils.ts
Expand Up @@ -47,6 +47,7 @@ export function getDefaultExpectations(): PaymentsManagerExpectations {
expected.addedVirtualCards = 0;
expected.requestedIbans = 1;
expected.removedIbans = 0;
expected.isValidIban = 0;
return expected;
}

Expand Down
1 change: 1 addition & 0 deletions extensions/browser/extension_function_histogram_value.h
Expand Up @@ -1831,6 +1831,7 @@ enum HistogramValue {
SMARTCARDPROVIDERPRIVATE_REPORTCONNECTRESULT = 1768,
SMARTCARDPROVIDERPRIVATE_REPORTDISCONNECTRESULT = 1769,
WMDESKSPRIVATE_GETDESKBYID = 1770,
AUTOFILLPRIVATE_ISVALIDIBAN = 1771,
// Last entry: Add new entries above, then run:
// tools/metrics/histograms/update_extension_histograms.py
ENUM_BOUNDARY
Expand Down
1 change: 1 addition & 0 deletions tools/metrics/histograms/enums.xml
Expand Up @@ -36182,6 +36182,7 @@ Called by update_extension_histograms.py.-->
<int value="1768" label="SMARTCARDPROVIDERPRIVATE_REPORTCONNECTRESULT"/>
<int value="1769" label="SMARTCARDPROVIDERPRIVATE_REPORTDISCONNECTRESULT"/>
<int value="1770" label="WMDESKSPRIVATE_GETDESKBYID"/>
<int value="1771" label="AUTOFILLPRIVATE_ISVALIDIBAN"/>
</enum>

<enum name="ExtensionIconState">
Expand Down
1 change: 1 addition & 0 deletions tools/typescript/definitions/autofill_private.d.ts
Expand Up @@ -123,6 +123,7 @@ declare global {
params: ValidatePhoneParams): Promise<string[]>;
export function getCreditCardList(): Promise<CreditCardEntry[]>;
export function getIbanList(): Promise<IbanEntry[]>;
export function isValidIban(ibanValue: string): Promise<boolean>;
export function maskCreditCard(guid: string): void;
export function migrateCreditCards(): void;
export function logServerCardLinkClicked(): void;
Expand Down

0 comments on commit 38e9918

Please sign in to comment.