From 527c1a7743bf5953621624c4f35962d1291fd6f2 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Tue, 12 Sep 2023 00:30:57 +0800 Subject: [PATCH] refactor(toolkit): update password policy --- .../core-kit/src/password-policy.test.ts | 151 +++++---- .../toolkit/core-kit/src/password-policy.ts | 313 +++++++++--------- 2 files changed, 249 insertions(+), 215 deletions(-) diff --git a/packages/toolkit/core-kit/src/password-policy.test.ts b/packages/toolkit/core-kit/src/password-policy.test.ts index 69c20e08b..018d41144 100644 --- a/packages/toolkit/core-kit/src/password-policy.test.ts +++ b/packages/toolkit/core-kit/src/password-policy.test.ts @@ -34,18 +34,33 @@ describe('PasswordPolicyChecker -> check()', () => { mockPwnResponse(); const checker = new PasswordPolicyChecker({ - length: { min: 7, max: 8 }, - characterTypes: { min: 2 }, + length: { min: 7, max: 15 }, + characterTypes: { min: 3 }, rejects: { pwned: true, repetitionAndSequence: true, userInfo: true, - words: ['aaaaaate', 'aaaaaaaa'], + words: ['aaaaaate', 'aaaaaaaa', 'silverhand'], }, }); it('should accept valid password', async () => { expect(await checker.check('aL1!aL1!', {})).toEqual([]); + expect(await checker.check('silverHAnd213', {})).toEqual([]); + expect(await checker.check('lo9KI8mJu112', {})).toEqual([]); + }); + + it('should recognize rejection combinations', async () => { + expect(await checker.check('aL1!aL1!', { name: 'aL1!' })).toEqual([ + { code: 'password_rejected.restricted.user_info' }, + ]); + expect(await checker.check('lo9KI8mJu78911', {})).toEqual([ + { code: 'password_rejected.restricted.sequence' }, + ]); + expect(await checker.check('lo9KI8mJu789111', {})).toEqual([ + { code: 'password_rejected.restricted.sequence' }, + { code: 'password_rejected.restricted.repetition' }, + ]); }); it('should reject with all failed checks', async () => { @@ -57,20 +72,19 @@ describe('PasswordPolicyChecker -> check()', () => { expect(await checker.check('123456', { phoneNumber: '12345' })).toEqual([ { code: 'password_rejected.too_short', interpolation: { min: 7 } }, - { code: 'password_rejected.character_types', interpolation: { min: 2 } }, + { code: 'password_rejected.character_types', interpolation: { min: 3 } }, { code: 'password_rejected.pwned' }, { code: 'password_rejected.restricted.sequence' }, { code: 'password_rejected.restricted.user_info' }, ]); - expect(await checker.check('aaaaaaaate', {})).toEqual([ - { code: 'password_rejected.too_long', interpolation: { max: 8 } }, - { code: 'password_rejected.character_types', interpolation: { min: 2 } }, + expect(await checker.check('aAaAaAaAaAaAaAaAaAteABcOK', { name: 'CO' })).toEqual([ + { code: 'password_rejected.too_long', interpolation: { max: 15 } }, + { code: 'password_rejected.character_types', interpolation: { min: 3 } }, { code: 'password_rejected.restricted.repetition' }, - { - code: 'password_rejected.restricted.words', - interpolation: { words: 'aaaaaate\naaaaaaaa', count: 2 }, - }, + { code: 'password_rejected.restricted.words' }, + { code: 'password_rejected.restricted.sequence' }, + { code: 'password_rejected.restricted.user_info' }, ]); }); }); @@ -122,61 +136,76 @@ describe('PasswordPolicyChecker -> hasBeenPwned()', () => { }); }); -describe('PasswordPolicyChecker -> hasRepetition()', () => { +describe('PasswordPolicyChecker -> repetitionLength()', () => { const checker = new PasswordPolicyChecker({ length: { min: 1, max: 256 }, characterTypes: { min: 2 }, rejects: { pwned: false, repetitionAndSequence: true, words: [] }, }); - it('should reject password with too many repeated characters', () => { - expect(checker.hasRepetition('aaaa')).toBe(true); - expect(checker.hasRepetition('aaa12')).toBe(true); - expect(checker.hasRepetition('aaaaaa😀')).toBe(true); + it('should recognize repeated characters that start at the beginning', () => { + expect(checker.repetitionLength('aaaa')).toBe(4); + expect(checker.repetitionLength('aaa12')).toBe(3); + expect(checker.repetitionLength('aaAaAa😀')).toBe(6); }); - 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!bbbbbbbbbbbb')).toBe(false); + it('should ignore repeated characters that do not start at the beginning or are too short', () => { + expect(checker.repetitionLength('a')).toBe(0); + expect(checker.repetitionLength('aa')).toBe(0); + expect(checker.repetitionLength('aL!bbbbb')).toBe(0); + expect(checker.repetitionLength('aL1!')).toBe(0); + expect(checker.repetitionLength('aL1!bbbbbbbbbbbb')).toBe(0); }); }); -describe('PasswordPolicyChecker -> hasUserInfo()', () => { +describe('PasswordPolicyChecker -> userInfoLength()', () => { const checker = new PasswordPolicyChecker({ rejects: { pwned: false, repetitionAndSequence: false, userInfo: true, words: [] }, }); - it('should reject password with name', () => { - expect(checker.hasUserInfo('test', { name: 'test' })).toBe(true); - 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 recognize name', () => { + expect(checker.userInfoLength('test', { name: 'test' })).toBe(4); + expect(checker.userInfoLength('test', { name: 'test2' })).toBe(0); + expect(checker.userInfoLength('FOO', { name: 'Foo bar' })).toBe(3); + expect(checker.userInfoLength('Foo', { name: 'bar fOo' })).toBe(3); + expect(checker.userInfoLength('barFooBaz12', { name: 'bar fOo baz' })).toBe(9); + expect(checker.userInfoLength('bar fOo baz12', { name: 'bar fOo baz' })).toBe(15); + expect(checker.userInfoLength('bar fOo baz12', { name: 'bar fOo baz' })).toBe(3); + expect(checker.userInfoLength('barfOo baz12', { name: 'bar fOo baz' })).toBe(3); }); - it('should reject password with username', () => { - expect(checker.hasUserInfo('1!test', { username: 'teST' })).toBe(true); - expect(checker.hasUserInfo('test123', { username: 'test2' })).toBe(false); + it('should recognize username', () => { + expect(checker.userInfoLength('test1!', { username: 'teST' })).toBe(4); + expect(checker.userInfoLength('test123', { username: 'test2' })).toBe(0); }); - it('should reject password with email', () => { - 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); + it('should recognize email', () => { + expect(checker.userInfoLength('teST1', { email: 'test@foo.com' })).toBe(4); + expect(checker.userInfoLength('TEST2', { email: 'test1@foo.com' })).toBe(0); + expect(checker.userInfoLength('FOO', { email: 'test@foo.com' })).toBe(0); + expect(checker.userInfoLength('Foo!', { email: 'fOO@foo.com' })).toBe(3); }); - it('should reject password with phone number', () => { - expect(checker.hasUserInfo('ST123456789', { phoneNumber: '123456789' })).toBe(true); - expect(checker.hasUserInfo('teST1234567890.', { phoneNumber: '123456789' })).toBe(false); - expect(checker.hasUserInfo('TEST12345678', { phoneNumber: '123456789' })).toBe(false); + it('should recognize phone number', () => { + expect(checker.userInfoLength('123456789ST', { phoneNumber: '123456789' })).toBe(9); + expect(checker.userInfoLength('123456789ST', { phoneNumber: '12' })).toBe(2); + expect(checker.userInfoLength('teST1234567890.', { phoneNumber: '123456789' })).toBe(0); + expect(checker.userInfoLength('TEST12345678', { phoneNumber: '123456789' })).toBe(0); + }); + + it('should return the longest match', () => { + expect( + checker.userInfoLength('123456789ST', { + name: '1234 56789', + username: '12345', + email: '1234567', + phoneNumber: '1234', + }) + ).toBe(9); }); }); -describe('PasswordPolicyChecker -> hasWords()', () => { +describe('PasswordPolicyChecker -> wordLength()', () => { const checker = new PasswordPolicyChecker({ length: { min: 1, max: 256 }, characterTypes: { min: 2 }, @@ -188,36 +217,38 @@ 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']); + it('should recognize blacklisted words (case insensitive)', () => { + expect(checker.wordLength('test')).toEqual(4); + expect(checker.wordLength('tEst2')).toEqual(5); }); - 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([]); + it('should ignore other words', () => { + expect(checker.wordLength('tes4')).toEqual(0); + expect(checker.wordLength('tes4 est5 tes t')).toEqual(0); + expect(checker.wordLength('tES4 TEst2 teSt3')).toEqual(0); }); }); -describe('PasswordPolicyChecker -> hasSequentialChars()', () => { +describe('PasswordPolicyChecker -> sequenceLength()', () => { const checker = new PasswordPolicyChecker({ length: { min: 1, max: 256 }, characterTypes: { min: 2 }, rejects: { pwned: false, repetitionAndSequence: true, userInfo: false, words: [] }, }); - it('should reject password with too many sequential characters', () => { - expect(checker.hasSequentialChars('1234')).toBe(true); - expect(checker.hasSequentialChars('edcba')).toBe(true); - expect(checker.hasSequentialChars('BCDEDC')).toBe(true); + it('should recognize string starts with too many sequential characters', () => { + expect(checker.sequenceLength('1234')).toBe(4); + expect(checker.sequenceLength('edcba')).toBe(5); + expect(checker.sequenceLength('BCDEDC')).toBe(4); + expect(checker.sequenceLength('yuIOp##')).toBe(5); + expect(checker.sequenceLength('2wsx3edc1')).toBe(4); + expect(checker.sequenceLength('lo9KI8mJu7890')).toBe(3); }); - 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); + it('should ignore string starts with too few sequential characters', () => { + expect(checker.sequenceLength('z')).toBe(0); + expect(checker.sequenceLength('FE')).toBe(0); + expect(checker.sequenceLength('aL1!')).toBe(0); + expect(checker.sequenceLength('aL1!BCDEFA')).toBe(0); }); }); diff --git a/packages/toolkit/core-kit/src/password-policy.ts b/packages/toolkit/core-kit/src/password-policy.ts index 5356e145d..cf580e963 100644 --- a/packages/toolkit/core-kit/src/password-policy.ts +++ b/packages/toolkit/core-kit/src/password-policy.ts @@ -45,7 +45,7 @@ export const passwordPolicyGuard = z.object({ .default({}), characterTypes: z .object({ - min: z.number().int().min(1).max(4).optional().default(2), + min: z.number().int().min(1).max(4).optional().default(1), }) .default({}), rejects: z @@ -110,6 +110,25 @@ export type UserInfo = Partial<{ */ export class PasswordPolicyChecker { static symbols = Object.freeze('!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~' as const); + + /** A set of characters that are considered as sequential. */ + static sequence = Object.freeze([ + '0123456789', + 'abcdefghijklmnopqrstuvwxyz', + 'qwertyuiop', + 'asdfghjkl', + 'zxcvbnm', + '1qaz', + '2wsx', + '3edc', + '4rfv', + '5tgb', + '6yhn', + '7ujm', + '8ik', + '9ol', + ] as const); + /** The length threshold for checking repetition and sequence. */ static repetitionAndSequenceThreshold = 3 as const; /** @@ -145,8 +164,7 @@ export class PasswordPolicyChecker { * @throws TypeError - If the policy requires to reject passwords that include user information * but the user information is not provided. */ - /* eslint-disable @silverhand/fp/no-mutating-methods */ - + /* eslint-disable @silverhand/fp/no-let, @silverhand/fp/no-mutation, @silverhand/fp/no-mutating-methods */ async check(password: string, userInfo?: UserInfo): Promise { const issues: PasswordIssue[] = this.fastCheck(password); @@ -156,43 +174,55 @@ export class PasswordPolicyChecker { }); } - if (this.policy.rejects.repetitionAndSequence) { - if (this.hasRepetition(password)) { - issues.push({ - code: 'password_rejected.restricted.repetition', - }); + // `hashArray[i]` indicates whether the `i`th character violates the restriction. + // We'll gradually set the value to `1` if needed. + // The algorithm time complexity should be O(n^2), but it's fast enough for a password. + const hashArray = Array.from<0 | 1>({ length: password.length }).fill(0); + const issueCodes = new Set(); + const { repetitionAndSequence, words, userInfo: rejectUserInfo } = this.policy.rejects; + const rejectWords = words.length > 0; + + const fillHashArray = (startIndex: number, length: number, code: PasswordRejectionCode) => { + if (length <= 0) { + return; } - if (this.hasSequentialChars(password)) { - issues.push({ - code: 'password_rejected.restricted.sequence', - }); + for (let i = startIndex; i < startIndex + length; i += 1) { + hashArray[i] = 1; + } + issueCodes.add(code); + }; + + for (let i = 0; i < password.length; i += 1) { + const sliced = password.slice(i); + + if (repetitionAndSequence) { + fillHashArray(i, this.repetitionLength(sliced), 'restricted.repetition'); + fillHashArray(i, this.sequenceLength(sliced), 'restricted.sequence'); + } + + if (rejectWords) { + fillHashArray(i, this.wordLength(sliced), 'restricted.words'); + } + + if (rejectUserInfo) { + if (!userInfo) { + throw new TypeError('User information data is required to check user information.'); + } + + fillHashArray(i, this.userInfoLength(sliced, userInfo), 'restricted.user_info'); } } - const words = this.hasWords(password); - - if (words.length > 0) { - issues.push({ - code: 'password_rejected.restricted.words', - interpolation: { words: words.join('\n'), count: words.length }, - }); - } - - if (this.policy.rejects.userInfo) { - if (!userInfo) { - throw new TypeError('User information is required to check user information.'); - } - - if (this.hasUserInfo(password, userInfo)) { - issues.push({ - code: 'password_rejected.restricted.user_info', - }); - } - } - - return issues; + return hashArray.reduce((total, current) => total + current, 0) > + PasswordPolicyChecker.getRestrictedPhraseThreshold(password) + ? [ + ...issues, + ...[...issueCodes].map((code) => ({ code: `password_rejected.${code}` })), + ] + : issues; } + /* eslint-enable @silverhand/fp/no-let, @silverhand/fp/no-mutation, @silverhand/fp/no-mutating-methods */ /** * Perform a fast check to see if the password passes the basic requirements. @@ -203,6 +233,7 @@ export class PasswordPolicyChecker { * @param password - Password to check. * @returns Whether the password passes the basic requirements. */ + /* eslint-disable @silverhand/fp/no-mutating-methods */ fastCheck(password: string) { const issues: PasswordIssue[] = []; @@ -282,171 +313,143 @@ export class PasswordPolicyChecker { } /** - * Check if the given password contains repetition characters that are more than - * the threshold. + * Get the length of the repetition at the beginning of the given string. + * For example, `repetitionLength('aaaaa')` will return `5`. * - * @param password - Password to check. - * @returns Whether the password contains repetition characters. + * If the length is less than {@link PasswordPolicyChecker.repetitionAndSequenceThreshold}, + * `0` will be returned. */ - hasRepetition(password: string): boolean { - const { repetitionAndSequenceThreshold, getRestrictedPhraseThreshold } = PasswordPolicyChecker; + /* eslint-disable @silverhand/fp/no-let, @silverhand/fp/no-mutation */ + repetitionLength(password: string): number { + const { repetitionAndSequenceThreshold } = PasswordPolicyChecker; + const firstChar = password[0]?.toLowerCase(); + let length = 0; - if (password.length < repetitionAndSequenceThreshold) { - return false; + for (const char of password) { + if (char.toLowerCase() === firstChar) { + length += 1; + } else { + break; + } } - const matchedRepetition = password.match( - new RegExp(`(.)\\1{${getRestrictedPhraseThreshold(password)},}`, 'g') - ); - - return Boolean(matchedRepetition); + return length >= repetitionAndSequenceThreshold ? length : 0; } + /* eslint-enable @silverhand/fp/no-let, @silverhand/fp/no-mutation */ /** - * Check if the given password contains user information. + * Get the length of the user information at the beginning of the given string. + * For example, `userInfoLength('silverhand', { username: 'silverhand' })` will return `10`. * - * @param password - Password to check. - * @param userInfo - User information to check. - * @returns Whether the password contains user information. + * For multiple matches, the longest length will be returned. */ // eslint-disable-next-line complexity - hasUserInfo(password: string, userInfo: UserInfo): boolean { - const lowercasedPassword = password.toLowerCase(); + userInfoLength(password: string, userInfo: UserInfo): number { + const lowercased = password.toLowerCase(); const { name, username, email, phoneNumber } = userInfo; - const threshold = PasswordPolicyChecker.getRestrictedPhraseThreshold(password); + // eslint-disable-next-line @silverhand/fp/no-let + let length = 0; + + const updateLength = (newLength: number) => { + if (newLength > length) { + // eslint-disable-next-line @silverhand/fp/no-mutation + length = newLength; + } + }; 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; + // The original name should be the longest string, so we check it first. + if (lowercased.startsWith(name.toLowerCase())) { + updateLength(name.length); + } else { + if (lowercased.startsWith(joined.toLowerCase())) { + updateLength(joined.length); + } + + for (const word of name.split(' ')) { + if (lowercased.startsWith(word.toLowerCase())) { + updateLength(word.length); + } + } } } - if ( - username && - username.length >= threshold && - lowercasedPassword.includes(username.toLowerCase()) - ) { - return true; + if (username && lowercased.startsWith(username.toLowerCase())) { + updateLength(username.length); } - const emailPrefix = email?.split('@')[0]; - if ( - emailPrefix && - emailPrefix.length >= threshold && - lowercasedPassword.includes(emailPrefix.toLowerCase()) - ) { - return true; + if (email) { + const emailPrefix = email.split('@')[0]; + if (emailPrefix && lowercased.startsWith(emailPrefix.toLowerCase())) { + updateLength(emailPrefix.length); + } } - if ( - phoneNumber && - phoneNumber.length >= threshold && - lowercasedPassword.includes(phoneNumber) - ) { - return true; + if (phoneNumber && lowercased.startsWith(phoneNumber)) { + updateLength(phoneNumber.length); } - return false; + return length; } /** - * Check if the given password contains specific words. + * Get the length of the word that matches the word list at the beginning of the given string. * - * @param password - Password to check. - * @returns An array of matched words. + * For multiple matches, the longest length will be returned. */ - hasWords(password: string): string[] { - const words = this.policy.rejects.words.map((word) => word.toLowerCase()); - const lowercasedPassword = password.toLowerCase(); - const threshold = PasswordPolicyChecker.getRestrictedPhraseThreshold(password); + /* eslint-disable @silverhand/fp/no-let, @silverhand/fp/no-mutation */ + wordLength(password: string): number { + const sliced = password.toLowerCase(); + let length = 0; - 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 the threshold. - * - * @param password - Password to check. - * @returns Whether the password contains sequential characters. - * - * @example - * ```ts - * hasSequentialChars('1'); // false - * hasSequentialChars('12'); // false - * hasSequentialChars('12345'); // true - * hasSequentialChars('123456@Bc.dcE'); // true - * hasSequentialChars('123@Bc.dcE'); // false - * ``` - */ - // 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; + for (const word of this.policy.rejects.words) { + if (sliced.startsWith(word.toLowerCase()) && word.length > length) { + length = word.length; + } } - const threshold = Math.max( - getRestrictedPhraseThreshold(password), - repetitionAndSequenceThreshold - ); - let sequence: number[] = []; + return length; + } + /* eslint-enable @silverhand/fp/no-let, @silverhand/fp/no-mutation */ + + /** + * Get the length of the sequence at the beginning of the given string. + * For example, `sequenceLength('12345')` will return `5`. + * + * If the length is less than {@link PasswordPolicyChecker.repetitionAndSequenceThreshold}, + * `0` will be returned. + */ + /* eslint-disable @silverhand/fp/no-let, @silverhand/fp/no-mutation */ + sequenceLength(password: string): number { + const { repetitionAndSequenceThreshold } = PasswordPolicyChecker; + let value = ''; + let length = 0; for (const char of password) { - // Always true - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const charCode = char.codePointAt(0)!; - - if (sequence.length === 0) { - sequence.push(charCode); - continue; - } - - if ( - this.checkSequentialWithChar(sequence, charCode, 1) || - this.checkSequentialWithChar(sequence, charCode, -1) - ) { - sequence.push(charCode); + if (!value || this.isSequential(value + char)) { + value += char; + length += 1; } else { - sequence = [charCode]; - } - - if (sequence.length >= threshold) { - return true; + break; } } - return false; + return length >= repetitionAndSequenceThreshold ? length : 0; } - /* eslint-enable @silverhand/fp/no-let, @silverhand/fp/no-mutation, @silverhand/fp/no-mutating-methods */ + /* eslint-enable @silverhand/fp/no-let, @silverhand/fp/no-mutation */ /** - * Check if the given char code will be sequential after appending to the given - * char code array. + * Check if the given string is sequential by iterating through the {@link PasswordPolicyChecker.sequence}. */ - protected checkSequentialWithChar( - current: Readonly, - newCharCode: number, - direction: 1 | -1 - ): boolean { - const lastCharCode = current.at(-1); + protected isSequential(value: string): boolean { + const { sequence } = PasswordPolicyChecker; + const lowercased = value.toLowerCase(); - if (newCharCode - direction === lastCharCode) { - if (current.length === 1) { - return true; - } - if (current.at(-2) === newCharCode - direction * 2) { + for (const seq of sequence) { + // eslint-disable-next-line @silverhand/fp/no-mutating-methods -- created a new array before mutating + if (seq.includes(lowercased) || [...seq].reverse().join('').includes(lowercased)) { return true; } }