Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 0 deletions .changeset/fix-password-policy-regex-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/password-policies': patch
---

Fixed the `forbidRepeatingCharactersCount` password policy silently turning itself off when configured with an invalid value. A negative, fractional, `NaN`, unsafely large or non-numeric count produced an invalid quantifier (`{-1,}`, `{1.5,}`, `{NaN,}`, `{1e+21,}`), which the regex engine reads as a literal, so passwords made entirely of repeated characters were accepted. A count of `0` had the opposite effect, rejecting every non-empty password and locking users out of setting one. Both cases now fall back to the default of `3`.
55 changes: 52 additions & 3 deletions packages/password-policies/src/PasswordPolicy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,57 @@ describe('Password Policy', () => {
const policy = passwordPolicy.getPasswordPolicy();

expect(policy.enabled).toBe(true);
// even when no policy is specified, forbidRepeatingCharactersCount is still configured
// since its default value is 3
expect(policy.policy.length).toBe(1);
// forbidRepeatingCharacters is disabled by default, so the count entry
// (and its default of 3) is not reported as an active policy
expect(policy.policy.length).toBe(0);
});

it.each([0, -1, 1.5, Number.NaN, 1e21, '3'])('should use the default repeating character count when configured with %p', (count) => {
const passwordPolicy = new PasswordPolicy({
enabled: true,
forbidRepeatingCharacters: true,
forbidRepeatingCharactersCount: count as number,
throwError: false,
});

// Default count is 3 → "111" is allowed, "1111" is not
expect(passwordPolicy.validate('111')).toBe(true);
expect(passwordPolicy.validate('1111')).toBe(false);

expect(passwordPolicy.sendValidationMessage('1111')).toContainEqual({
name: 'get-password-policy-forbidRepeatingCharactersCount',
isValid: false,
limit: 3,
});

expect(passwordPolicy.getPasswordPolicy().policy).toContainEqual([
'get-password-policy-forbidRepeatingCharactersCount',
{ forbidRepeatingCharactersCount: 3 },
]);
});

it('should not lock every password out when configured with a count of 0', () => {
const passwordPolicy = new PasswordPolicy({
enabled: true,
forbidRepeatingCharacters: true,
forbidRepeatingCharactersCount: 0,
throwError: false,
});

// `(.)\1{0,}` matches any non-empty string, so a count of 0 would reject every password
expect(passwordPolicy.validate('1')).toBe(true);
expect(passwordPolicy.validate('Passw0rd!')).toBe(true);
});

it('should keep enforcing the rule when configured with an unusable count', () => {
const passwordPolicy = new PasswordPolicy({
enabled: true,
forbidRepeatingCharacters: true,
forbidRepeatingCharactersCount: -1,
throwError: false,
});

// a `{-1,}` quantifier is read as a literal, which would silently disable the rule
expect(passwordPolicy.validate('11111111')).toBe(false);
});
});
11 changes: 7 additions & 4 deletions packages/password-policies/src/PasswordPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,24 @@ export class PasswordPolicy {
mustContainAtLeastOneSpecialCharacter = false,
throwError = true,
}: PasswordPolicyOptions) {
// Invalid counts interpolate as literals rather than throwing, so the rule
// would silently stop matching; `{0,}` matches every non-empty password.
const safeForbidRepeatingCharactersCount =
Number.isSafeInteger(forbidRepeatingCharactersCount) && forbidRepeatingCharactersCount >= 1 ? forbidRepeatingCharactersCount : 3;

this.enabled = enabled;
this.minLength = minLength;
this.maxLength = maxLength;
this.forbidRepeatingCharacters = forbidRepeatingCharacters;
this.forbidRepeatingCharactersCount = forbidRepeatingCharactersCount;
this.forbidRepeatingCharactersCount = safeForbidRepeatingCharactersCount;
this.mustContainAtLeastOneLowercase = mustContainAtLeastOneLowercase;
this.mustContainAtLeastOneUppercase = mustContainAtLeastOneUppercase;
this.mustContainAtLeastOneNumber = mustContainAtLeastOneNumber;
this.mustContainAtLeastOneSpecialCharacter = mustContainAtLeastOneSpecialCharacter;
this.throwError = throwError;

this.regex = {
forbiddingRepeatingCharacters: new RegExp(`(.)\\1{${forbidRepeatingCharactersCount},}`),
forbiddingRepeatingCharacters: new RegExp(`(.)\\1{${safeForbidRepeatingCharactersCount},}`),
mustContainAtLeastOneLowercase: new RegExp('[a-z]'),
mustContainAtLeastOneUppercase: new RegExp('[A-Z]'),
mustContainAtLeastOneNumber: new RegExp('[0-9]'),
Expand Down Expand Up @@ -262,8 +267,6 @@ export class PasswordPolicy {
}
if (this.forbidRepeatingCharacters) {
data.policy.push(['get-password-policy-forbidRepeatingCharacters']);
}
if (this.forbidRepeatingCharactersCount) {
data.policy.push([
'get-password-policy-forbidRepeatingCharactersCount',
{ forbidRepeatingCharactersCount: this.forbidRepeatingCharactersCount },
Expand Down