Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing password strength check #1071

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion assets/locales/ur/auth.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"EMAIL_INVALID": "Invalid Email",
"EMAIL_ALREADY_REGISTERED": "Email is already registered",
"DATE_OF_BIRTH_UNDERAGE": "You need to be {{years}} years or older",
"PASSWORD_REQUIREMENTS_MIN_LENGTH": "Must be at least {{min}} characters long.",
"PASSWORD_BAD_PASSWORD": "Password is too weak or common to use.",
"CONSENT_REQUIRED": "You must agree to the Terms of Service and Privacy Policy.",
"USERNAME_TOO_MANY_USERS": "Too many users have this username, please try another"
}
Expand Down
14 changes: 4 additions & 10 deletions src/api/routes/auth/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import {
IPAnalysis,
checkPassword,
getIpAdress,
isProxy,
route,
Expand Down Expand Up @@ -159,7 +160,6 @@ router.post(
}

// TODO: gift_code_sku_id?
// TODO: check password strength

if (email) {
// replace all dots and chars after +, if its a gmail.com email
Expand Down Expand Up @@ -225,17 +225,11 @@ router.post(
}

if (body.password) {
const min = register.password.minLength
? register.password.minLength
: 8;
if (body.password.length < min) {
if (checkPassword(body.password) < register.password.strength) {
throw FieldErrors({
password: {
code: "PASSWORD_REQUIREMENTS_MIN_LENGTH",
message: req.t(
"auth:register.PASSWORD_REQUIREMENTS_MIN_LENGTH",
{ min: min },
),
code: "PASSWORD_BAD_PASSWORD",
message: req.t("auth:register.PASSWORD_BAD_PASSWORD"),
},
});
}
Expand Down
45 changes: 25 additions & 20 deletions src/api/util/utility/passwordStrength.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import { Config } from "@spacebar/util";
import "missing-native-js-functions";

const reNUMBER = /[0-9]/g;
const reUPPERCASELETTER = /[A-Z]/g;
const reSYMBOLS = /[A-Z,a-z,0-9]/g;
const reUPPER = /[A-Z]/g;
const reSYMBOLS = /[^a-zA-Z0-9\s]/g;

// const blocklist: string[] = []; // TODO: update ones passwordblocklist is stored in db
/*
Expand All @@ -36,36 +36,41 @@ const reSYMBOLS = /[A-Z,a-z,0-9]/g;
* Returns: 0 > pw > 1
*/
export function checkPassword(password: string): number {
const { minLength, minNumbers, minUpperCase, minSymbols } =
Config.get().register.password;
let strength = 0;
const { strength } = Config.get().register.password;

let pwStrength = 0;

// checks for total password len
if (password.length >= minLength - 1) {
strength += 0.05;
if (password.length >= 7) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these might been hard coded values for min and max that should have been configurable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I add the configuration options back then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I add the configuration options back then?

So far, we've tried to make as much as possible configurable my the instance owner, and I think the password policy would be no exception. But probably wait for other opinions on it.

pwStrength += 0.05;
}

// checks for amount of Numbers
if (password.count(reNUMBER) >= minNumbers - 1) {
strength += 0.05;
const numbers = password.match(reNUMBER);
if (numbers && numbers.length >= 1) {
pwStrength += 0.05;
}

// checks for amount of Uppercase Letters
if (password.count(reUPPERCASELETTER) >= minUpperCase - 1) {
strength += 0.05;
const uppercase = password.match(reUPPER);
if (uppercase && uppercase.length >= 1) {
pwStrength += 0.05;
}

// checks for amount of symbols
if (password.replace(reSYMBOLS, "").length >= minSymbols - 1) {
strength += 0.05;
const symbols = password.match(reSYMBOLS);
if (symbols && symbols.length >= 1) {
pwStrength += 0.05;
}

// checks if password only consists of numbers or only consists of chars
if (
password.length == password.count(reNUMBER) ||
password.length === password.count(reUPPERCASELETTER)
) {
strength = 0;
if (numbers && uppercase) {
if (
password.length == numbers.length ||
password.length === uppercase.length
) {
pwStrength = 0;
}
}

const entropyMap: { [key: string]: number } = {};
Expand All @@ -77,8 +82,8 @@ export function checkPassword(password: string): number {
const entropies = Object.values(entropyMap);

entropies.map((x) => x / entropyMap.length);
strength +=
pwStrength +=
entropies.reduceRight((a: number, x: number) => a - x * Math.log2(x)) /
Math.log2(password.length);
return strength;
return pwStrength;
}
5 changes: 1 addition & 4 deletions src/util/config/types/subconfigurations/register/Password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,5 @@

export class PasswordConfiguration {
required: boolean = false;
minLength: number = 8;
minNumbers: number = 2;
minUpperCase: number = 2;
minSymbols: number = 0;
strength: number = 0.5;
}