fix(client,server): usernames should not be a http error code (#37804)

* fix(client,server): usernames should not be a http error code
* feat: reject invalid chars first

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

api-server/server/boot/settings.js

@@ -4,7 +4,7 @@ import { check } from 'express-validator/check';
 import { ifNoUser401, createValidatorErrorHandler } from '../utils/middleware';
 import { themes } from '../../common/utils/themes.js';
 import { alertTypes } from '../../common/utils/flash.js';
-import { validate } from '../../../utils/validate';
+import { isValidUsername } from '../../../utils/validate';
 
 const log = debug('fcc:boot:settings');
 
@@ -200,7 +200,7 @@ function createUpdateMyUsername(app) {
         message: 'Username is already associated with this account'
       });
     }
-    const validation = validate(username);
+    const validation = isValidUsername(username);
 
     if (!validation.valid) {
       return res.json({

client/src/components/settings/Username.js

@@ -17,7 +17,7 @@ import {
 } from '../../redux/settings';
 import BlockSaveButton from '../helpers/form/BlockSaveButton';
 import FullWidthRow from '../helpers/FullWidthRow';
-import { validate } from '../../../../utils/validate';
+import { isValidUsername } from '../../../../utils/validate';
 
 const propTypes = {
   isValidUsername: PropTypes.bool,
@@ -112,7 +112,7 @@ class UsernameSettings extends Component {
   }
 
   validateFormInput(formValue) {
-    return validate(formValue);
+    return isValidUsername(formValue);
   }
 
   renderAlerts(validating, error, isValidUsername) {

utils/validate.js

@@ -1,13 +1,31 @@
-const validCharsRE = /^[a-zA-Z0-9\-_+]+$/;
 const invalidCharError = {
   valid: false,
-  error: 'contains invalid characters'
+  error: 'contains invalid characters.'
 };
 const validationSuccess = { valid: true, error: null };
-const usernameTooShort = { valid: false, error: 'is too short' };
+const usernameTooShort = { valid: false, error: 'is too short.' };
+const usernameIsHttpStatusCode = {
+  valid: false,
+  error: 'is a reserved error code.'
+};
 
-exports.validate = str => {
-  if (str.length < 3) return usernameTooShort;
+const isNumeric = num => !isNaN(num);
+const validCharsRE = /^[a-zA-Z0-9\-_+]*$/;
+const isHttpStatusCode = str =>
+  isNumeric(str) && (parseInt(str, 10) >= 100 && parseInt(str, 10) <= 599);
+const isValidUsername = str => {
   if (!validCharsRE.test(str)) return invalidCharError;
+  if (str.length < 3) return usernameTooShort;
+  if (isHttpStatusCode(str)) return usernameIsHttpStatusCode;
   return validationSuccess;
 };
+
+module.exports = {
+  isNumeric,
+  isHttpStatusCode,
+  isValidUsername,
+  validationSuccess,
+  usernameTooShort,
+  usernameIsHttpStatusCode,
+  invalidCharError
+};

utils/validate.test.js

@@ -1,36 +1,42 @@
 /* global describe expect it */
 
-const { validate } = require('./validate');
-
-const invalidCharError = {
-  valid: false,
-  error: 'contains invalid characters'
-};
-const validationSuccess = { valid: true, error: null };
-const usernameTooShort = { valid: false, error: 'is too short' };
+const {
+  isValidUsername,
+  usernameTooShort,
+  validationSuccess,
+  usernameIsHttpStatusCode,
+  invalidCharError
+} = require('./validate');
 
 function inRange(num, range) {
   return num >= range[0] && num <= range[1];
 }
 
-describe('validate', () => {
+describe('isValidUsername', () => {
   it('rejects strings with less than 3 characters', () => {
-    expect(validate('')).toStrictEqual(usernameTooShort);
-    expect(validate('12')).toStrictEqual(usernameTooShort);
-    expect(validate('a')).toStrictEqual(usernameTooShort);
-    expect(validate('123')).toStrictEqual(validationSuccess);
+    expect(isValidUsername('')).toStrictEqual(usernameTooShort);
+    expect(isValidUsername('12')).toStrictEqual(usernameTooShort);
+    expect(isValidUsername('a')).toStrictEqual(usernameTooShort);
+    expect(isValidUsername('12a')).toStrictEqual(validationSuccess);
+  });
+  it('rejects strings which are http response status codes 100-599', () => {
+    expect(isValidUsername('429')).toStrictEqual(usernameIsHttpStatusCode);
+    expect(isValidUsername('789')).toStrictEqual(validationSuccess);
   });
   it('rejects non-ASCII characters', () => {
-    expect(validate('👀👂👄')).toStrictEqual(invalidCharError);
+    expect(isValidUsername('👀👂👄')).toStrictEqual(invalidCharError);
+  });
+  it('rejects with invalidCharError even if the string is too short', () => {
+    expect(isValidUsername('.')).toStrictEqual(invalidCharError);
   });
   it('accepts alphanumeric characters', () => {
-    expect(validate('abcdefghijklmnopqrstuvwxyz0123456789')).toStrictEqual(
-      validationSuccess
-    );
+    expect(
+      isValidUsername('abcdefghijklmnopqrstuvwxyz0123456789')
+    ).toStrictEqual(validationSuccess);
   });
   it('accepts hyphens and underscores', () => {
-    expect(validate('a-b')).toStrictEqual(validationSuccess);
-    expect(validate('a_b')).toStrictEqual(validationSuccess);
+    expect(isValidUsername('a-b')).toStrictEqual(validationSuccess);
+    expect(isValidUsername('a_b')).toStrictEqual(validationSuccess);
   });
 
   it('rejects all other ASCII characters', () => {
@@ -48,7 +54,7 @@ describe('validate', () => {
       if (inRange(code, numbers)) expected = validationSuccess;
       if (inRange(code, upperCase)) expected = validationSuccess;
       if (inRange(code, lowerCase)) expected = validationSuccess;
-      expect(validate(base + char)).toStrictEqual(expected);
+      expect(isValidUsername(base + char)).toStrictEqual(expected);
     }
   });
 });