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.

Sign up for GitHub

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
Open

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
276f31e
Refactor the PINValidation for adjacent numbers repetition and releva…
jian4on Jan 18, 2025
8f4d738
Fixed typo in commments
jian4on Jan 18, 2025
a0fd478
Resolved the 'Cognitive Complexity' issue in code scan
jian4on Jan 18, 2025
013a819
Removed the 4-times repetition rule, fixed test failures
jian4on Jan 18, 2025
5c412ff
Changed the no_repeated_numbers PIN rule to max_repeated_numbers and …
jian4on Jan 21, 2025
354f500
Fixed test
jian4on Jan 21, 2025
bafa89c
Restore the default no_repeated_numbers behavior, but keep the new va…
jian4on Jan 22, 2025
fc6edee
Changed the adjacent PIN number repetition validation to a number typ…
jian4on Feb 3, 2025
9cadf77
Merge branch 'main' into jian4on/did-2011
al-rosenthal Feb 3, 2025
005976c
Refactor the repetition validating rule, set '0' as 'disable repetiti…
jian4on Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/legacy/core/App/container-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ import { InlineErrorPosition } from './types/error'
import { DefaultScreenLayoutOptions } from './navigators/defaultLayoutOptions'

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
6 changes: 6 additions & 0 deletions packages/legacy/core/App/types/security.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ export enum AuthLevel {
BiometricsOnly = 'BiometricsOnly',
}

/*
no_repeated_numbers - forbid adjacent numbers repeating or not, and specify the forbidden repeating times when repeating allowed
true | 0 | 1 | 2: forbid adjacent numbers repetition
false: no check for adjacent numbers repeating
other numbers: the length of adjacent repeating numbers is forbidden, e.g. '3' will forbid 055567, but 055678 is allowed
*/
export interface PINValidationRules {
only_numbers: boolean
min_length: number
Expand Down
22 changes: 15 additions & 7 deletions packages/legacy/core/App/utils/PINCreationValidation.ts
View file
Open in desktop
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 ('number' === typeof(PINRules.no_repeated_numbers) && PINRules.no_repeated_numbers > 2 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When would the > 2 check be needed at the same time as the string interpolation below is? Wouldn't there only be one case (3) where that condition is satisfied?

const repetitionPattern = new RegExp(String.raw`(\d)\1{${PINRules.no_repeated_numbers - 1},}`, "g")
PINValidations.push({
isInvalid: noRepeatedNumbers.test(PIN),
errorName: PINError.NoRepetitionOfTheSameNumbersValidation,
isInvalid: repetitionPattern.test(PIN),
errorName: PINError.MaxAdjacentNumberRepetitionValidation
} as PINValidationsType)
} else if (PINRules.no_repeated_numbers || 0 === PINRules.no_repeated_numbers) {
const repetitionPattern = new RegExp(/(\d)\1{1,}/)
PINValidations.push({
isInvalid: repetitionPattern.test(PIN),
errorName: PINError.NoRepetitionOfTheSameNumbersValidation
} 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
2 changes: 1 addition & 1 deletion packages/legacy/core/__tests__/utils/PINCreationValidation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ 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 () => {
const PINRulesWithRepeatedNumbers = {
...defaultPINRules,
no_repeated_numbers: true,
no_repeated_numbers: 0,
Copy link
Contributor

@jleach jleach Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think no_ should be change to num_ if it now indicates the number of of repeating values allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jleach Good point, It is type 'boolean | number' now. I changed it to an enum [1,2,3,4 ...] type in the Draft version for discussing and then changed it back. Actually I still think maxRepetition number is an option.
@bryce-mcmath If you think we need more discussion, I will pull this feat back to draft

Copy link
Contributor

Choose a reason for hiding this comment

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

@jian4on ah right, the type changed. num is good I think, we can change the default since the type is changing. maybe just add a note in the "Breaking changes" section of the description of this PR so other teams know what to change to keep their existing behavior

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this will work well. There will be no way to disable the rule unless another flag is added. You can put in a high number but that will then be displayed during password selection, which would be confusing for the user.

}

const PINWithRepeatedNumbers = '113456'
Expand Down