Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve the PIN Enter repetition validation rule and add relevant messages #1401

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion packages/legacy/core/App/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const PINRules: PINValidationRules = {
only_numbers: true,
min_length: 6,
max_length: 6,
no_repeated_numbers: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is in draft but if it's doable could we keep the defaults the same and just allow for opt-in behavior with the injectable PINSecurity.rules config we have currently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put both PINSecurity and PINCreationValidation to IoC container, then the wallet variants can customize them without changinf Bifold default validating behavior. I will discuss with team. Thanks @bryce-mcmath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back to default, but change the validating logic for '0' as 'repeating times' instead of its boolean value, keep the extra message for numbers repeating allowed but the repeating times exceeding the max times rule

no_repeated_numbers: 0,
no_repetition_of_the_two_same_numbers: false,
no_series_of_numbers: false,
no_even_or_odd_series_of_numbers: false,
Expand Down
5 changes: 4 additions & 1 deletion packages/legacy/core/App/container-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ import { DefaultScreenLayoutOptions } from './navigators/defaultLayoutOptions'
import ConnectionAlert from './components/misc/ConnectionAlert'

export const defaultConfig: Config = {
PINSecurity: { rules: PINRules, displayHelper: false },
PINSecurity: {
rules: PINRules,
displayHelper: false,
},
settings: [],
enableChat: true,
enableTours: false,
Expand Down
2 changes: 2 additions & 0 deletions packages/legacy/core/App/localization/en/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ const translation = {
"CrossPatternValidation": "A pattern of cross was detected in your PIN. Please try again.",
"OddOrEvenSequenceValidation": "An odd or even sequence was detected in your PIN. Please try again.",
"NoRepetitionOfTheSameNumbersValidation": "The PIN can't have a repetition of the same digit. Please try again.",
"MaxAdjacentNumberRepetitionValidation": "Adjacent numbers repeating times are more than allowed. Please try again.",
"NoRepetitionOfTheTwoSameNumbersValidation": "A repeated two-digit sequence was detected in your PIN. Please try again.",
"NoSeriesOfNumbersValidation": "A series was detected in your PIN. Please try again.",
"PINOnlyContainDigitsValidation": "Your PIN needs to only contain digits. Please try again.",
Expand All @@ -217,6 +218,7 @@ const translation = {
"CrossPatternValidation": "Does not contain a pattern of cross.",
"OddOrEvenSequenceValidation": "Does not contain an odd or even sequence.",
"NoRepetitionOfTheSameNumbersValidation": "Does not contain the same repeating number.",
"MaxAdjacentNumberRepetitionValidation": "Adjacent numbers repeating times are limited.",
"NoRepetitionOfTheTwoSameNumbersValidation": "Does not contain repeating two-digit sequence.",
"NoSeriesOfNumbersValidation": "Does not contain a series of numbers (123).",
"PINOnlyContainDigitsValidation": "Must contain only digits.",
Expand Down
2 changes: 2 additions & 0 deletions packages/legacy/core/App/localization/fr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ const translation = {
"Message": {
"CrossPatternValidation": "Un patron de croix a été détecté dans votre NIP. Veuillez saisir un NIP différent.",
"OddOrEvenSequenceValidation": "Une séquence impaire ou paire a été détectée dans votre NIP. Veuillez saisir un NIP différent.",
"MaxAdjacentNumberRepetitionValidation": "Adjacent numbers repeating times are more than allowed. Please try again. (FR)",
"NoRepetitionOfTheTwoSameNumbersValidation": "Une séquence de deux chiffres répétée a été détectée dans votre NIP. Veuillez saisir un NIP différent.",
"NoRepetitionOfTheSameNumbersValidation": "Votre NIP contient une répétition. Choisissez un NIP différent.",
"NoSeriesOfNumbersValidation": "Une suite a été détectée dans votre NIP. Veuillez saisir un NIP différent.",
Expand All @@ -215,6 +216,7 @@ const translation = {
"Helper": {
"CrossPatternValidation": "Ne doit pas contenir un patron de croix.",
"OddOrEvenSequenceValidation": "Ne doit pas contenir une suite paire ou impaire.",
"MaxAdjacentNumberRepetitionValidation": "Adjacent numbers repeating times are limited. (FR)",
"NoRepetitionOfTheTwoSameNumbersValidation": "Ne doit pas contenir deux chiffres qui se répètent.",
"NoRepetitionOfTheSameNumbersValidation": "Ne doit pas contenir de répétition.",
"NoSeriesOfNumbersValidation": "Ne doit pas contenir de suite (123).",
Expand Down
2 changes: 2 additions & 0 deletions packages/legacy/core/App/localization/pt-br/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ const translation = {
"CrossPatternValidation": "Um padrão de X foi detectado no seu PIN. Favor tentar novamente.",
"OddOrEvenSequenceValidation": "Uma sequencia de números pares ou ímpares foi detectada em seu PIN. Favor tentar novamente.",
"NoRepetitionOfTheSameNumbersValidation": "O PIN não pode ter a repetição de um mesmo número. Favor tentar novamente.",
"MaxAdjacentNumberRepetitionValidation": "Adjacent numbers repeating times are more than allowed. Please try again. (PT-BR)",
"NoRepetitionOfTheTwoSameNumbersValidation": "Uma sequência repetida de dois números foi detectada em seu PIN. Favor tentar novamente.",
"NoSeriesOfNumbersValidation": "Uma sequência foi detectada em seu PIN. Favor tentar novamente.",
"PINOnlyContainDigitsValidation": "Seu PIN deve conter apenas números. Favor tentar novamente.",
Expand All @@ -210,6 +211,7 @@ const translation = {
"CrossPatternValidation": "Não contém um padrão de X.",
"OddOrEvenSequenceValidation": "Não contém uma sequência de números pares ou ímpares.",
"NoRepetitionOfTheSameNumbersValidation": "Não contém o mesmo número repetido.",
"MaxAdjacentNumberRepetitionValidation": "Adjacent numbers repeating times are limited. (PT-BR)",
"NoRepetitionOfTheTwoSameNumbersValidation": "Não contém uma sequência repetida de dois números.",
"NoSeriesOfNumbersValidation": "Não contém uma sequência de números (123).",
"PINOnlyContainDigitsValidation": "Deve conter apenas números.",
Expand Down
7 changes: 6 additions & 1 deletion packages/legacy/core/App/types/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ export enum AuthLevel {
BiometricsOnly = 'BiometricsOnly',
}

/*
no_repeated_numbers - adjacent characters allowed repeating times
0 - Disable adjacent number repeating validation, any times repeating are allowed
n > 0 - Enable the repeating validation, n repeating times are forbidden, e.g. n = 2 forbid repeating 2 times, '11' is allowed but '111' forbidden
*/
export interface PINValidationRules {
only_numbers: boolean
min_length: number
max_length: number
no_repeated_numbers: boolean | number
no_repeated_numbers: number
no_repetition_of_the_two_same_numbers: boolean | number
Comment on lines +22 to 23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably make the same change for the rules below:

Suggested change
no_repeated_numbers: number
no_repetition_of_the_two_same_numbers: boolean | number
no_repeated_numbers: number
no_repetition_of_the_two_same_numbers: number

no_series_of_numbers: boolean
no_even_or_odd_series_of_numbers: boolean
Expand Down
22 changes: 15 additions & 7 deletions packages/legacy/core/App/utils/PINCreationValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export enum PINError {
CrossPatternValidation = 'CrossPatternValidation',
OddOrEvenSequenceValidation = 'OddOrEvenSequenceValidation',
NoRepetitionOfTheSameNumbersValidation = 'NoRepetitionOfTheSameNumbersValidation',
MaxAdjacentNumberRepetitionValidation = 'MaxAdjacentNumberRepetitionValidation',
NoRepetitionOfTheTwoSameNumbersValidation = 'NoRepetitionOfTheTwoSameNumbersValidation',
NoSeriesOfNumbersValidation = 'NoSeriesOfNumbersValidation',
PINOnlyContainDigitsValidation = 'PINOnlyContainDigitsValidation',
Expand All @@ -24,6 +25,7 @@ export interface PINValidationsType {

export const PINCreationValidations = (PIN: string, PINRules: PINValidationRules) => {
const PINValidations: PINValidationsType[] = []

if (PINRules.no_cross_pattern) {
PINValidations.push({
isInvalid: crossNumberPattern.includes(PIN),
Expand All @@ -36,16 +38,21 @@ export const PINCreationValidations = (PIN: string, PINRules: PINValidationRules
errorName: PINError.OddOrEvenSequenceValidation,
} as PINValidationsType)
}
if (PINRules.no_repeated_numbers) {
let noRepeatedNumbers = new RegExp(/(\d)\1{1,}/)
if (typeof PINRules.no_repeated_numbers === 'number') {
noRepeatedNumbers = new RegExp(`(\\d)\\1{${PINRules.no_repeated_numbers - 1},}`)
}

if (1 == PINRules.no_repeated_numbers) {
const repetitionPattern = new RegExp(/(\d)\1{1,}/)
PINValidations.push({
isInvalid: noRepeatedNumbers.test(PIN),
errorName: PINError.NoRepetitionOfTheSameNumbersValidation,
isInvalid: repetitionPattern.test(PIN),
errorName: PINError.NoRepetitionOfTheSameNumbersValidation
} as PINValidationsType)
} else if (1 < PINRules.no_repeated_numbers){
const repetitionPattern = new RegExp(String.raw`(\d)\1{${PINRules.no_repeated_numbers},}`, "g")
PINValidations.push({
isInvalid: repetitionPattern.test(PIN),
errorName: PINError.MaxAdjacentNumberRepetitionValidation
} as PINValidationsType)
}

if (PINRules.no_repetition_of_the_two_same_numbers) {
let noRepetitionOfTheTwoSameNumbers = new RegExp(/([0-9][0-9])\1{1,}/)
if (typeof PINRules.no_repetition_of_the_two_same_numbers === 'number') {
Expand All @@ -58,6 +65,7 @@ export const PINCreationValidations = (PIN: string, PINRules: PINValidationRules
errorName: PINError.NoRepetitionOfTheTwoSameNumbersValidation,
} as PINValidationsType)
}

if (PINRules.no_series_of_numbers) {
PINValidations.push({
isInvalid: consecutiveSeriesOfThree.test(PIN),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const defaultPINRules = {
only_numbers: false,
min_length: 6,
max_length: 6,
no_repeated_numbers: false,
no_repeated_numbers: 0,
no_series_of_numbers: false,
no_repetition_of_the_two_same_numbers: false,
no_even_or_odd_series_of_numbers: false,
Expand Down Expand Up @@ -44,6 +44,17 @@ describe('PIN creation validations', () => {
}
})

test('PIN adjacent number repetion is allowed when the repeating test is disabled', async () => {
const PINRepeating = '111111'
const PINValidations = PINCreationValidations(PINRepeating, defaultPINRules)

for (const PINValidation of PINValidations) {
if (PINValidation.errorName === 'PINTooLongValidation') {
expect(PINValidation.isInvalid).toBe(false)
}
}
})

test('PIN without only number and only number validation to true should return PINOnlyContainDigitsValidation as invalid', async () => {
const PINRulesWithOnlyNumbers = {
...defaultPINRules,
Expand Down Expand Up @@ -74,10 +85,10 @@ describe('PIN creation validations', () => {
}
})

test('PIN with repeated numbers and repeated numbers validation to true, so the validation use the default of two repeated numbers, should return NoRepetitionOfTheSameNumbersValidation as invalid', async () => {
test('PIN with repeated numbers and no_repeated_numbers is 1, should return NoRepetitionOfTheSameNumbersValidation as invalid', async () => {
const PINRulesWithRepeatedNumbers = {
...defaultPINRules,
no_repeated_numbers: true,
no_repeated_numbers: 1,
}

const PINWithRepeatedNumbers = '113456'
Expand All @@ -90,10 +101,10 @@ describe('PIN creation validations', () => {
}
})

test('PIN with repeated numbers and repeated numbers validation to a number, so the validation use that number as validation limit, should return NoRepetitionOfTheSameNumbersValidation as invalid', async () => {
test('PIN with 3 repeating adhacent numbers and no_repeated_numbers is 2, so the validation use that number as validation max limit, should return NoRepetitionOfTheSameNumbersValidation as invalid', async () => {
const PINRulesWithRepeatedNumbers = {
...defaultPINRules,
no_repeated_numbers: 3,
max_repeated_numbers: 2,
}

const PINWithRepeatedNumbers = '111456'
Expand All @@ -106,13 +117,13 @@ describe('PIN creation validations', () => {
}
})

test('PIN with a numbers repeated two times and repeated numbers validation set to 3 should return NoRepetitionOfTheSameNumbersValidation as valid', async () => {
test('PIN with 3 repeating adhacent numbers and no_repeated_numbers is 3, so the validation use that number as validation max limit, should return NoRepetitionOfTheSameNumbersValidation as valid', async () => {
const PINRulesWithRepeatedNumbers = {
...defaultPINRules,
no_repeated_numbers: 3,
max_repeated_numbers: 2,
}

const PINWithRepeatedNumbers = '112456'
const PINWithRepeatedNumbers = '111456'
const PINValidations = PINCreationValidations(PINWithRepeatedNumbers, PINRulesWithRepeatedNumbers)

for (const PINValidation of PINValidations) {
Expand Down