Skip to content

Commit

Permalink
[PM-11588] Bugfix - parse user input value for combined expiry date w…
Browse files Browse the repository at this point in the history
…hen creating/adding a card cipher (#11103)

* simplify logic and fix some pattern-matching bugs

* add first pass at parsing combined expiry year and month from user input

* clean up code

* fix broken three-digit parsing case

* fix case where splitCombinedDateValues returns empty strings when the input is only a delimiter

* fix incorrect expectation of falsy negative integers

* clean up code

* split out logic from parseYearMonthExpiry

* move utils from vault to autofill
  • Loading branch information
jprusik authored Sep 24, 2024
1 parent c8084cc commit e88e231
Show file tree
Hide file tree
Showing 16 changed files with 648 additions and 222 deletions.
15 changes: 13 additions & 2 deletions apps/browser/src/autofill/background/overlay.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
import { parseYearMonthExpiry } from "@bitwarden/common/autofill/utils";
import { NeverDomains } from "@bitwarden/common/models/domain/domain-service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import {
Expand Down Expand Up @@ -1898,11 +1899,21 @@ export class OverlayBackground implements OverlayBackgroundInterface {
const cardView = new CardView();
cardView.cardholderName = card.cardholderName || "";
cardView.number = card.number || "";
cardView.expMonth = card.expirationMonth || "";
cardView.expYear = card.expirationYear || "";
cardView.code = card.cvv || "";
cardView.brand = card.number ? CardView.getCardBrandByPatterns(card.number) : "";

// If there's a combined expiration date value and no individual month or year values,
// try to parse them from the combined value
if (card.expirationDate && !card.expirationMonth && !card.expirationYear) {
const [parsedYear, parsedMonth] = parseYearMonthExpiry(card.expirationDate);

cardView.expMonth = parsedMonth || "";
cardView.expYear = parsedYear || "";
} else {
cardView.expMonth = card.expirationMonth || "";
cardView.expYear = card.expirationYear || "";
}

const cipherView = new CipherView();
cipherView.name = "";
cipherView.folderId = null;
Expand Down
2 changes: 0 additions & 2 deletions apps/browser/src/autofill/services/autofill-constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,6 @@ export class CreditCardAutoFillConstants {
"cb-type",
];

static readonly CardExpiryDateDelimiters: string[] = ["/", "-", ".", " "];

// Note, these are expressions of user-guidance for the expected expiry date format to be used
static readonly CardExpiryDateFormats: CardExpiryDateFormat[] = [
// English
Expand Down
15 changes: 9 additions & 6 deletions apps/browser/src/autofill/services/autofill.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants";
import {
AutofillOverlayVisibility,
CardExpiryDateDelimiters,
} from "@bitwarden/common/autofill/constants";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
import { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service";
import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types";
import { normalizeExpiryYearFormat } from "@bitwarden/common/autofill/utils";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { EventType } from "@bitwarden/common/enums";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
Expand All @@ -30,7 +34,6 @@ import { CardView } from "@bitwarden/common/vault/models/view/card.view";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { FieldView } from "@bitwarden/common/vault/models/view/field.view";
import { IdentityView } from "@bitwarden/common/vault/models/view/identity.view";
import { normalizeExpiryYearFormat } from "@bitwarden/common/vault/utils";

import { BrowserApi } from "../../platform/browser/browser-api";
import { ScriptInjectorService } from "../../platform/services/abstractions/script-injector.service";
Expand Down Expand Up @@ -1397,8 +1400,7 @@ export default class AutofillService implements AutofillServiceInterface {
if (expectedExpiryDateFormat) {
const { Month, MonthShort, Year } = expiryDateFormatPatterns;

const expiryDateDelimitersPattern =
"\\" + CreditCardAutoFillConstants.CardExpiryDateDelimiters.join("\\");
const expiryDateDelimitersPattern = "\\" + CardExpiryDateDelimiters.join("\\");

// assign the delimiter from the expected format string
delimiter =
Expand Down Expand Up @@ -1450,8 +1452,7 @@ export default class AutofillService implements AutofillServiceInterface {
let expectedDateFormat = null;
let dateFormatPatterns = null;

const expiryDateDelimitersPattern =
"\\" + CreditCardAutoFillConstants.CardExpiryDateDelimiters.join("\\");
const expiryDateDelimitersPattern = "\\" + CardExpiryDateDelimiters.join("\\");

CreditCardAutoFillConstants.CardExpiryDateFormats.find((dateFormat) => {
dateFormatPatterns = dateFormat;
Expand Down Expand Up @@ -1489,6 +1490,8 @@ export default class AutofillService implements AutofillServiceInterface {
return false;
});
});
// @TODO if expectedDateFormat is still null, and there is a `pattern` attribute, cycle
// through generated formatted values, checking against the provided regex pattern

return [expectedDateFormat, dateFormatPatterns];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { normalizeExpiryYearFormat } from "@bitwarden/common/autofill/utils";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
Expand All @@ -23,7 +24,6 @@ import { CollectionService } from "@bitwarden/common/vault/abstractions/collecti
import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
import { CipherType } from "@bitwarden/common/vault/enums";
import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view";
import { normalizeExpiryYearFormat } from "@bitwarden/common/vault/utils";
import { DialogService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { EventCollectionService } from "@bitwarden/common/abstractions/event/eve
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { isCardExpired } from "@bitwarden/common/autofill/utils";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { ProductTierType } from "@bitwarden/common/billing/enums";
import { EventType } from "@bitwarden/common/enums";
Expand All @@ -24,7 +25,6 @@ import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folde
import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { Launchable } from "@bitwarden/common/vault/interfaces/launchable";
import { isCardExpired } from "@bitwarden/common/vault/utils";
import { DialogService } from "@bitwarden/components";
import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";
import { PasswordRepromptService } from "@bitwarden/vault";
Expand Down
2 changes: 1 addition & 1 deletion libs/angular/src/vault/components/add-edit.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
import { OrganizationUserStatusType, PolicyType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { normalizeExpiryYearFormat } from "@bitwarden/common/autofill/utils";
import { EventType } from "@bitwarden/common/enums";
import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
Expand All @@ -36,7 +37,6 @@ import { IdentityView } from "@bitwarden/common/vault/models/view/identity.view"
import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view";
import { LoginView } from "@bitwarden/common/vault/models/view/login.view";
import { SecureNoteView } from "@bitwarden/common/vault/models/view/secure-note.view";
import { normalizeExpiryYearFormat } from "@bitwarden/common/vault/utils";
import { DialogService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";

Expand Down
2 changes: 2 additions & 0 deletions libs/common/src/autofill/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,5 @@ export type ExtensionCommandType = (typeof ExtensionCommand)[keyof typeof Extens
export const CLEAR_NOTIFICATION_LOGIN_DATA_DURATION = 60 * 1000; // 1 minute

export const MAX_DEEP_QUERY_RECURSION_DEPTH = 4;

export * from "./match-patterns";
26 changes: 26 additions & 0 deletions libs/common/src/autofill/constants/match-patterns.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
export const CardExpiryDateDelimiters: string[] = ["/", "-", ".", " "];

// `CardExpiryDateDelimiters` is not intended solely for regex consumption,
// so we need to format it here
export const ExpiryDateDelimitersPattern =
"\\" +
CardExpiryDateDelimiters.join("\\")
// replace space character with the regex whitespace character class
.replace(" ", "s");

export const MonthPattern = "(([1]{1}[0-2]{1})|(0?[1-9]{1}))";

// Because we're dealing with expiry dates, we assume the year will be in current or next century (as of 2024)
export const ExpiryFullYearPattern = "2[0-1]{1}\\d{2}";

export const DelimiterPatternExpression = new RegExp(`[${ExpiryDateDelimitersPattern}]`, "g");

export const IrrelevantExpiryCharactersPatternExpression = new RegExp(
// "nor digits" to ensure numbers are removed from guidance pattern, which aren't covered by ^\w
`[^\\d${ExpiryDateDelimitersPattern}]`,
"g",
);

export const MonthPatternExpression = new RegExp(`^${MonthPattern}$`);

export const ExpiryFullYearPatternExpression = new RegExp(`^${ExpiryFullYearPattern}$`);
Loading

0 comments on commit e88e231

Please sign in to comment.