From f71763da2599b322fad8e3101e84e7ab7014553a Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Wed, 6 Sep 2023 23:34:56 +0800 Subject: [PATCH 1/2] refactor(toolkit): sync latest password rules --- .../core-kit/src/password-policy.test.ts | 42 ++++++---- .../toolkit/core-kit/src/password-policy.ts | 82 +++++++++++++++---- 2 files changed, 91 insertions(+), 33 deletions(-) diff --git a/packages/toolkit/core-kit/src/password-policy.test.ts b/packages/toolkit/core-kit/src/password-policy.test.ts index 657a6d0e8..69c20e08b 100644 --- a/packages/toolkit/core-kit/src/password-policy.test.ts +++ b/packages/toolkit/core-kit/src/password-policy.test.ts @@ -40,7 +40,7 @@ describe('PasswordPolicyChecker -> check()', () => { pwned: true, repetitionAndSequence: true, userInfo: true, - words: ['test', 'aaaa'], + words: ['aaaaaate', 'aaaaaaaa'], }, }); @@ -49,9 +49,9 @@ describe('PasswordPolicyChecker -> check()', () => { }); it('should reject with all failed checks', async () => { - expect(await checker.check('aaa', {})).toEqual([ + expect(await checker.check('aaa😀', {})).toEqual([ { code: 'password_rejected.too_short', interpolation: { min: 7 } }, - { code: 'password_rejected.character_types', interpolation: { min: 2 } }, + { code: 'password_rejected.unsupported_characters' }, { code: 'password_rejected.restricted.repetition' }, ]); @@ -63,13 +63,13 @@ describe('PasswordPolicyChecker -> check()', () => { { code: 'password_rejected.restricted.user_info' }, ]); - expect(await checker.check('aaaaaatest😀', {})).toEqual([ + expect(await checker.check('aaaaaaaate', {})).toEqual([ { code: 'password_rejected.too_long', interpolation: { max: 8 } }, - { code: 'password_rejected.unsupported_characters' }, + { code: 'password_rejected.character_types', interpolation: { min: 2 } }, { code: 'password_rejected.restricted.repetition' }, { code: 'password_rejected.restricted.words', - interpolation: { words: 'test\naaaa', count: 2 }, + interpolation: { words: 'aaaaaate\naaaaaaaa', count: 2 }, }, ]); }); @@ -131,13 +131,16 @@ describe('PasswordPolicyChecker -> hasRepetition()', () => { it('should reject password with too many repeated characters', () => { expect(checker.hasRepetition('aaaa')).toBe(true); - expect(checker.hasRepetition('aL1!bbbbb')).toBe(true); - expect(checker.hasRepetition('aaaaaatest😀')).toBe(true); + expect(checker.hasRepetition('aaa12')).toBe(true); + expect(checker.hasRepetition('aaaaaa😀')).toBe(true); }); it('should accept password with few repeated characters', () => { + expect(checker.hasRepetition('a')).toBe(false); + expect(checker.hasRepetition('aa')).toBe(false); + expect(checker.hasRepetition('aL!bbbbb')).toBe(false); expect(checker.hasRepetition('aL1!')).toBe(false); - expect(checker.hasRepetition('aL1!bbbb')).toBe(false); + expect(checker.hasRepetition('aL1!bbbbbbbbbbbb')).toBe(false); }); }); @@ -151,22 +154,24 @@ describe('PasswordPolicyChecker -> hasUserInfo()', () => { expect(checker.hasUserInfo('test', { name: 'test2' })).toBe(false); expect(checker.hasUserInfo('FOO', { name: 'Foo bar' })).toBe(true); expect(checker.hasUserInfo('Foo', { name: 'bar fOo' })).toBe(true); + expect(checker.hasUserInfo('barFooBaz12', { name: 'bar fOo baz' })).toBe(true); }); it('should reject password with username', () => { - expect(checker.hasUserInfo('123.456!test', { username: 'teST' })).toBe(true); - expect(checker.hasUserInfo('test', { username: 'test2' })).toBe(false); + expect(checker.hasUserInfo('1!test', { username: 'teST' })).toBe(true); + expect(checker.hasUserInfo('test123', { username: 'test2' })).toBe(false); }); it('should reject password with email', () => { - expect(checker.hasUserInfo('teST', { email: 'test@foo.com' })).toBe(true); - expect(checker.hasUserInfo('TEST', { email: 'test1@foo.com' })).toBe(false); + expect(checker.hasUserInfo('teST1', { email: 'test@foo.com' })).toBe(true); + expect(checker.hasUserInfo('TEST2', { email: 'test1@foo.com' })).toBe(false); expect(checker.hasUserInfo('FOO', { email: 'test@foo.com' })).toBe(false); - expect(checker.hasUserInfo('Foo', { email: 'fOO@foo.com' })).toBe(true); + expect(checker.hasUserInfo('Foo!', { email: 'fOO@foo.com' })).toBe(true); }); it('should reject password with phone number', () => { - expect(checker.hasUserInfo('teST1234567890.', { phoneNumber: '123456789' })).toBe(true); + expect(checker.hasUserInfo('ST123456789', { phoneNumber: '123456789' })).toBe(true); + expect(checker.hasUserInfo('teST1234567890.', { phoneNumber: '123456789' })).toBe(false); expect(checker.hasUserInfo('TEST12345678', { phoneNumber: '123456789' })).toBe(false); }); }); @@ -186,12 +191,12 @@ describe('PasswordPolicyChecker -> hasWords()', () => { it('should reject password with blacklisted words (case insensitive)', () => { expect(checker.hasWords('test')).toEqual(['test']); expect(checker.hasWords('tEst2')).toEqual(['test', 'test2']); - expect(checker.hasWords('tEST TEst2 teSt3')).toEqual(['test', 'test2', 'test3']); }); it('should accept password without blacklisted words', () => { expect(checker.hasWords('tes4')).toEqual([]); expect(checker.hasWords('tes4 est5 tes t')).toEqual([]); + expect(checker.hasWords('tEST TEst2 teSt3')).toEqual([]); }); }); @@ -203,15 +208,16 @@ describe('PasswordPolicyChecker -> hasSequentialChars()', () => { }); it('should reject password with too many sequential characters', () => { - expect(checker.hasSequentialChars('FE')).toBe(true); expect(checker.hasSequentialChars('1234')).toBe(true); expect(checker.hasSequentialChars('edcba')).toBe(true); - expect(checker.hasSequentialChars('aL1!BCDEFA')).toBe(true); + expect(checker.hasSequentialChars('BCDEDC')).toBe(true); }); it('should accept password with few sequential characters', () => { expect(checker.hasSequentialChars('z')).toBe(false); + expect(checker.hasSequentialChars('FE')).toBe(false); expect(checker.hasSequentialChars('aL1!')).toBe(false); + expect(checker.hasSequentialChars('aL1!BCDEFA')).toBe(false); expect(checker.hasSequentialChars('aL1!bcdedcb1234321')).toBe(false); }); }); diff --git a/packages/toolkit/core-kit/src/password-policy.ts b/packages/toolkit/core-kit/src/password-policy.ts index e8063155b..5356e145d 100644 --- a/packages/toolkit/core-kit/src/password-policy.ts +++ b/packages/toolkit/core-kit/src/password-policy.ts @@ -110,6 +110,20 @@ export type UserInfo = Partial<{ */ export class PasswordPolicyChecker { static symbols = Object.freeze('!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~' as const); + /** The length threshold for checking repetition and sequence. */ + static repetitionAndSequenceThreshold = 3 as const; + /** + * If the password contains more than such number of characters that are not + * in the restricted phrases, it will be accepted. + */ + static restrictedPhrasesTolerance = 3 as const; + + /** Get the length threshold for checking restricted phrases. */ + protected static getRestrictedPhraseThreshold(password: string): number { + const { restrictedPhrasesTolerance } = PasswordPolicyChecker; + + return Math.max(1, password.length - restrictedPhrasesTolerance); + } public readonly policy: PasswordPolicy; @@ -269,14 +283,20 @@ export class PasswordPolicyChecker { /** * Check if the given password contains repetition characters that are more than - * 4 characters or equal to the password length. + * the threshold. * * @param password - Password to check. * @returns Whether the password contains repetition characters. */ hasRepetition(password: string): boolean { + const { repetitionAndSequenceThreshold, getRestrictedPhraseThreshold } = PasswordPolicyChecker; + + if (password.length < repetitionAndSequenceThreshold) { + return false; + } + const matchedRepetition = password.match( - new RegExp(`(.)\\1{${Math.min(password.length - 1, 4)},}`, 'g') + new RegExp(`(.)\\1{${getRestrictedPhraseThreshold(password)},}`, 'g') ); return Boolean(matchedRepetition); @@ -289,29 +309,50 @@ export class PasswordPolicyChecker { * @param userInfo - User information to check. * @returns Whether the password contains user information. */ + // eslint-disable-next-line complexity hasUserInfo(password: string, userInfo: UserInfo): boolean { const lowercasedPassword = password.toLowerCase(); const { name, username, email, phoneNumber } = userInfo; + const threshold = PasswordPolicyChecker.getRestrictedPhraseThreshold(password); + + if (name) { + const joined = name.replaceAll(/\s+/g, ''); + if (joined.length >= threshold && lowercasedPassword.includes(joined.toLowerCase())) { + return true; + } + + if ( + name + .toLowerCase() + .split(' ') + .some((word) => word.length >= threshold && lowercasedPassword.includes(word)) + ) { + return true; + } + } if ( - name - ?.toLowerCase() - .split(' ') - .some((word) => lowercasedPassword.includes(word)) + username && + username.length >= threshold && + lowercasedPassword.includes(username.toLowerCase()) ) { return true; } - if (username && lowercasedPassword.includes(username.toLowerCase())) { - return true; - } - const emailPrefix = email?.split('@')[0]; - if (emailPrefix && lowercasedPassword.includes(emailPrefix.toLowerCase())) { + if ( + emailPrefix && + emailPrefix.length >= threshold && + lowercasedPassword.includes(emailPrefix.toLowerCase()) + ) { return true; } - if (phoneNumber && lowercasedPassword.includes(phoneNumber)) { + if ( + phoneNumber && + phoneNumber.length >= threshold && + lowercasedPassword.includes(phoneNumber) + ) { return true; } @@ -327,13 +368,14 @@ export class PasswordPolicyChecker { hasWords(password: string): string[] { const words = this.policy.rejects.words.map((word) => word.toLowerCase()); const lowercasedPassword = password.toLowerCase(); + const threshold = PasswordPolicyChecker.getRestrictedPhraseThreshold(password); - return words.filter((word) => lowercasedPassword.includes(word)); + return words.filter((word) => word.length >= threshold && lowercasedPassword.includes(word)); } /** * Check if the given password contains sequential characters, and the number of - * sequential characters is over 4 or equal to the password length when (1 < length < 5). + * sequential characters is over the threshold. * * @param password - Password to check. * @returns Whether the password contains sequential characters. @@ -341,6 +383,7 @@ export class PasswordPolicyChecker { * @example * ```ts * hasSequentialChars('1'); // false + * hasSequentialChars('12'); // false * hasSequentialChars('12345'); // true * hasSequentialChars('123456@Bc.dcE'); // true * hasSequentialChars('123@Bc.dcE'); // false @@ -349,6 +392,15 @@ export class PasswordPolicyChecker { // Disable the mutation rules because the algorithm is much easier to implement with /* eslint-disable @silverhand/fp/no-let, @silverhand/fp/no-mutation, @silverhand/fp/no-mutating-methods */ hasSequentialChars(password: string): boolean { + const { repetitionAndSequenceThreshold, getRestrictedPhraseThreshold } = PasswordPolicyChecker; + if (password.length < repetitionAndSequenceThreshold) { + return false; + } + + const threshold = Math.max( + getRestrictedPhraseThreshold(password), + repetitionAndSequenceThreshold + ); let sequence: number[] = []; for (const char of password) { @@ -370,7 +422,7 @@ export class PasswordPolicyChecker { sequence = [charCode]; } - if (sequence.length >= 5 || sequence.length === password.length) { + if (sequence.length >= threshold) { return true; } } From b68a58178904a03685fa1f16aee0ac026c4a58d9 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Thu, 7 Sep 2023 22:42:52 +0800 Subject: [PATCH 2/2] refactor(ui): refine password input --- .../src/Layout/SecondaryPageLayout/index.tsx | 2 +- .../ui/src/containers/SetPassword/index.tsx | 1 + packages/ui/src/hooks/use-sie.ts | 28 +++++++++---------- .../src/pages/Continue/SetPassword/index.tsx | 5 ++-- .../ui/src/pages/RegisterPassword/index.tsx | 5 ++-- packages/ui/src/pages/ResetPassword/index.tsx | 5 ++-- 6 files changed, 25 insertions(+), 21 deletions(-) diff --git a/packages/ui/src/Layout/SecondaryPageLayout/index.tsx b/packages/ui/src/Layout/SecondaryPageLayout/index.tsx index 978a8ac25..f5a5899ae 100644 --- a/packages/ui/src/Layout/SecondaryPageLayout/index.tsx +++ b/packages/ui/src/Layout/SecondaryPageLayout/index.tsx @@ -12,7 +12,7 @@ import * as styles from './index.module.scss'; type Props = { title: TFuncKey; - description?: TFuncKey | ReactElement; + description?: TFuncKey | ReactElement | ''; titleProps?: Record; descriptionProps?: Record; notification?: TFuncKey; diff --git a/packages/ui/src/containers/SetPassword/index.tsx b/packages/ui/src/containers/SetPassword/index.tsx index d78b5f355..bdb07f582 100644 --- a/packages/ui/src/containers/SetPassword/index.tsx +++ b/packages/ui/src/containers/SetPassword/index.tsx @@ -10,6 +10,7 @@ type Props = { onSubmit: (password: string) => void; errorMessage?: string; clearErrorMessage?: () => void; + maxLength?: number; }; const SetPassword = (props: Props) => { diff --git a/packages/ui/src/hooks/use-sie.ts b/packages/ui/src/hooks/use-sie.ts index 8beb39adb..4af0ab3d0 100644 --- a/packages/ui/src/hooks/use-sie.ts +++ b/packages/ui/src/hooks/use-sie.ts @@ -42,20 +42,20 @@ export const usePasswordPolicy = () => { const requirementsDescription = useMemo( () => - t('description.password_requirements', { - items: condArray( - // There's no need to show the length requirement if it can be satisfied by the character types - policy.length.min > policy.characterTypes.min && - t('description.password_requirement.length', { count: policy.length.min }), - // Show the character types requirement if: - // - It's greater than 1, or; - // - The length requirement is equal to or less than 1 (since the length requirement will be hidden in this case) - (policy.characterTypes.min > 1 || policy.length.min <= 1) && - t('description.password_requirement.character_types', { - count: policy.characterTypes.min, - }) - ), - }), + policy.length.min <= 1 && policy.characterTypes.min <= 1 + ? undefined + : t('description.password_requirements', { + items: condArray( + // There's no need to show the length requirement if it can be satisfied by the character types + policy.length.min > policy.characterTypes.min && + t('description.password_requirement.length', { count: policy.length.min }), + // Show the character types requirement if it's greater than 1. + policy.characterTypes.min > 1 && + t('description.password_requirement.character_types', { + count: policy.characterTypes.min, + }) + ), + }), [t, policy.length.min, policy.characterTypes.min] ); diff --git a/packages/ui/src/pages/Continue/SetPassword/index.tsx b/packages/ui/src/pages/Continue/SetPassword/index.tsx index 1b19b184e..aea74c9fd 100644 --- a/packages/ui/src/pages/Continue/SetPassword/index.tsx +++ b/packages/ui/src/pages/Continue/SetPassword/index.tsx @@ -44,7 +44,7 @@ const SetPassword = () => { const { policy: { - length: { min }, + length: { min, max }, characterTypes: { min: count }, }, requirementsDescription, @@ -53,12 +53,13 @@ const SetPassword = () => { return ( {requirementsDescription}} + description={requirementsDescription && {requirementsDescription}} descriptionProps={{ min, count }} > diff --git a/packages/ui/src/pages/RegisterPassword/index.tsx b/packages/ui/src/pages/RegisterPassword/index.tsx index 78b9bcaf1..051139581 100644 --- a/packages/ui/src/pages/RegisterPassword/index.tsx +++ b/packages/ui/src/pages/RegisterPassword/index.tsx @@ -46,7 +46,7 @@ const RegisterPassword = () => { const { policy: { - length: { min }, + length: { min, max }, characterTypes: { min: count }, }, requirementsDescription, @@ -59,12 +59,13 @@ const RegisterPassword = () => { return ( {requirementsDescription}} + description={requirementsDescription && {requirementsDescription}} descriptionProps={{ min, count }} > diff --git a/packages/ui/src/pages/ResetPassword/index.tsx b/packages/ui/src/pages/ResetPassword/index.tsx index 604c46986..90d1e5310 100644 --- a/packages/ui/src/pages/ResetPassword/index.tsx +++ b/packages/ui/src/pages/ResetPassword/index.tsx @@ -50,7 +50,7 @@ const ResetPassword = () => { }); const { policy: { - length: { min }, + length: { min, max }, characterTypes: { min: count }, }, requirementsDescription, @@ -59,12 +59,13 @@ const ResetPassword = () => { return ( {requirementsDescription}} + description={requirementsDescription && {requirementsDescription}} descriptionProps={{ min, count }} >