Skip to content
Permalink
Browse files

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>
  • Loading branch information
raisedadead and ojeytonwilliams committed Nov 27, 2019
1 parent b794908 commit 9886cf7ca2f9e99c97ab9530295926b064840abd
Showing with 53 additions and 29 deletions.
  1. +2 −2 api-server/server/boot/settings.js
  2. +2 −2 client/src/components/settings/Username.js
  3. +23 −5 utils/validate.js
  4. +26 −20 utils/validate.test.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({
@@ -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) {
@@ -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
};
@@ -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);
}
});
});

0 comments on commit 9886cf7

Please sign in to comment.
You can’t perform that action at this time.