From f8c2ec0e9cd949d991901c3aa5e8b9b343bf760e Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Tue, 2 Aug 2022 10:23:31 +0200 Subject: [PATCH 1/9] login limiter done single window and brute forcing one user --- .../curator-service/api/package-lock.json | 18 +++ verification/curator-service/api/package.json | 1 + .../api/src/controllers/auth.ts | 112 ++++++++++++++++-- .../api/src/model/failed_attempts.ts | 88 ++++++++++++++ 4 files changed, 209 insertions(+), 10 deletions(-) create mode 100644 verification/curator-service/api/src/model/failed_attempts.ts diff --git a/verification/curator-service/api/package-lock.json b/verification/curator-service/api/package-lock.json index 06e5f459f..a05cd9d77 100644 --- a/verification/curator-service/api/package-lock.json +++ b/verification/curator-service/api/package-lock.json @@ -22,6 +22,7 @@ "envalid": "^7.2.2", "express": "^4.17.1", "express-openapi-validator": "^4.9.0", + "express-rate-limit": "^6.5.1", "express-session": "^1.17.1", "express-winston": "^4.2.0", "i18n-iso-countries": "^7.3.0", @@ -4188,6 +4189,17 @@ "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-6.2.0.tgz", "integrity": "sha512-f66KywYG6+43afgE/8j/GoiNyygk/bnoCbps++3ErRKsIYkGGupyv07R2Ok5m9i67Iqc+T2g1eAUGUPzWhYTyg==" }, + "node_modules/express-rate-limit": { + "version": "6.5.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-6.5.1.tgz", + "integrity": "sha512-pxO6ioBLd3i8IHL+RmJtL4noYzte5fugoMdaDabtU4hcg53+x0QkTwfPtM7vWD0YUaXQgNj9NRdzmps+CHEHlA==", + "engines": { + "node": ">= 12.9.0" + }, + "peerDependencies": { + "express": "^4 || ^5" + } + }, "node_modules/express-session": { "version": "1.17.2", "resolved": "https://registry.npmjs.org/express-session/-/express-session-1.17.2.tgz", @@ -13369,6 +13381,12 @@ } } }, + "express-rate-limit": { + "version": "6.5.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-6.5.1.tgz", + "integrity": "sha512-pxO6ioBLd3i8IHL+RmJtL4noYzte5fugoMdaDabtU4hcg53+x0QkTwfPtM7vWD0YUaXQgNj9NRdzmps+CHEHlA==", + "requires": {} + }, "express-session": { "version": "1.17.2", "resolved": "https://registry.npmjs.org/express-session/-/express-session-1.17.2.tgz", diff --git a/verification/curator-service/api/package.json b/verification/curator-service/api/package.json index dc9efe34b..c3d5a6676 100644 --- a/verification/curator-service/api/package.json +++ b/verification/curator-service/api/package.json @@ -77,6 +77,7 @@ "envalid": "^7.2.2", "express": "^4.17.1", "express-openapi-validator": "^4.9.0", + "express-rate-limit": "^6.5.1", "express-session": "^1.17.1", "express-winston": "^4.2.0", "i18n-iso-countries": "^7.3.0", diff --git a/verification/curator-service/api/src/controllers/auth.ts b/verification/curator-service/api/src/controllers/auth.ts index e9eb3c201..392612353 100644 --- a/verification/curator-service/api/src/controllers/auth.ts +++ b/verification/curator-service/api/src/controllers/auth.ts @@ -23,6 +23,25 @@ import * as crypto from 'crypto'; import EmailClient from '../clients/email-client'; import { ObjectId } from 'mongodb'; import { baseURL, welcomeEmail } from '../util/instance-details'; +import rateLimit from 'express-rate-limit'; +import { + failed_attempts, + setupFailedAttempts, + handleCheckFailedAttempts, +} from '../model/failed_attempts'; + +const loginLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, // 60 minutes + max: 4, // Limit each IP to 4 requests per `window` (here, per 60 minutes) + standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers + legacyHeaders: false, // Disable the `X-RateLimit-*` headers + handler: function (req, res /*next*/) { + return res.status(429).json({ + message: + 'You sent too many requests. Please wait a while then try again', + }); + }, +}); // Global variable for newsletter acceptance let isNewsletterAccepted: boolean; @@ -214,16 +233,26 @@ export class AuthController { this.router.post( '/signin', + loginLimiter, (req: Request, res: Response, next: NextFunction): void => { passport.authenticate( 'login', - (error: Error, user: IUser, info: any) => { + ( + error: Error, + user: IUser & { timeout: boolean }, + info: any, + ) => { if (error) return next(error); if (!user) return res .status(403) .json({ message: info.message }); + if (user.timeout) + return res + .status(429) + .json({ message: info.message }); + req.logIn(user, (err) => { if (err) return next(err); }); @@ -427,7 +456,12 @@ export class AuthController { try { // Check if user with this email address exists - const user = await users().findOne({ email }); + const userPromise = await users() + .find({ email: email }) + .collation({ locale: 'en_US', strength: 2 }) + .toArray(); + + const user = userPromise[0] as IUser; if (!user) { return res.sendStatus(200); } @@ -603,6 +637,9 @@ export class AuthController { const user = (await users().findOne({ _id: result.insertedId, })) as IUser; + + setupFailedAttempts(result.insertedId); + req.login(user, (err: Error) => { if (!err) { res.json(user); @@ -660,12 +697,13 @@ export class AuthController { }, async (req, email, password, done) => { try { - const userPromise = await users().find({ email }) - .collation({ locale: 'en_US', strength: 2 }) - .toArray(); + const userPromise = await users() + .find({ email }) + .collation({ locale: 'en_US', strength: 2 }) + .toArray(); const user = userPromise[0]; - + if (user) { return done(null, false, { message: 'Email address already exists', @@ -688,6 +726,8 @@ export class AuthController { _id: result.insertedId, })) as IUser; + setupFailedAttempts(result.insertedId); + // Send welcome email await this.emailClient.send( [email], @@ -712,9 +752,10 @@ export class AuthController { }, async (email, password, done) => { try { - const userPromise = await users().find({ email }) - .collation({ locale: 'en_US', strength: 2 }) - .toArray(); + const userPromise = await users() + .find({ email }) + .collation({ locale: 'en_US', strength: 2 }) + .toArray(); const user = userPromise[0] as IUser; @@ -724,16 +765,56 @@ export class AuthController { }); } + const { success, attemptsNumber } = + await handleCheckFailedAttempts( + user._id, + 'loginAttempt', + ); + + if (!success) + return done( + null, + { timeout: true }, + { + message: + 'Too Many Attempts try it one hour later', + }, + ); + const isValidPassword = await isUserPasswordValid( user, password, ); + if (!isValidPassword) { + await failed_attempts().updateOne( + { userId: user._id }, + { + $set: { + loginAttempt: { + count: attemptsNumber, + createdAt: new Date(), + }, + }, + }, + ); + return done(null, false, { message: 'Wrong username or password', }); } + await failed_attempts().updateOne( + { userId: user._id }, + { + $set: { + loginAttempt: { + count: 0, + createdAt: new Date(), + }, + }, + }, + ); done(null, user); } catch (error) { done(error); @@ -781,6 +862,8 @@ export class AuthController { _id: result.insertedId, })) as IUser; + setupFailedAttempts(result.insertedId); + try { // Send welcome email await this.emailClient.send( @@ -865,7 +948,13 @@ export class AuthController { 'Supplied bearer token must be scoped for "email"', ); } - let user = await users().findOne({ email: email }); + const userPromise = await users() + .find({ email: email }) + .collation({ locale: 'en_US', strength: 2 }) + .toArray(); + + let user = userPromise[0] as IUser | null; + if (!user) { const result = await users().insertOne({ _id: new ObjectId(), @@ -878,7 +967,10 @@ export class AuthController { user = await users().findOne({ _id: result.insertedId, }); + + setupFailedAttempts(result.insertedId); } + return done(null, user); } catch (e) { return done(e); diff --git a/verification/curator-service/api/src/model/failed_attempts.ts b/verification/curator-service/api/src/model/failed_attempts.ts new file mode 100644 index 000000000..15f9742d2 --- /dev/null +++ b/verification/curator-service/api/src/model/failed_attempts.ts @@ -0,0 +1,88 @@ +import { Collection, ObjectId } from 'mongodb'; +import db from './database'; + +/* + * This is a minimal case schema to support some source-related behaviour. + * The full schema for cases is in the data service. + */ + +const maxNumberOfFailedLogins = 8; +const timeWindowForFailedLogins = 60; //min + +type attemptName = 'loginAttempt' | 'registerAttempt' | 'resetPasswordAttempt'; + +export type IfailedAttempts = { + _id: ObjectId; + userId: ObjectId; + loginAttempt: { + count: number; + createdAt: Date; + }; + registerAttempt: { + count: number; + createdAt: Date; + }; + resetPasswordAttempt: { + count: number; + createdAt: Date; + }; +}; + +export const failed_attempts = () => + db().collection('failed_attempts') as Collection; + +export const setupFailedAttempts = async (userId: ObjectId) => { + await failed_attempts().insertOne({ + _id: new ObjectId(), + userId: userId, + loginAttempt: { + count: 0, + createdAt: new Date(), + }, + registerAttempt: { + count: 0, + createdAt: new Date(), + }, + resetPasswordAttempt: { + count: 0, + createdAt: new Date(), + }, + }); +}; + +export const handleCheckFailedAttempts = async ( + userId: ObjectId, + attemptName: attemptName, +) => { + const attempts = (await failed_attempts().findOne({ + userId: userId, + })) as IfailedAttempts; + + let attemptsNumber = attempts[attemptName].count + 1; + + const diffTimeMin = + Math.floor( + Math.abs(Date.now() - attempts[attemptName].createdAt.getTime()) / + 1000, + ) / 60; + + if (diffTimeMin >= timeWindowForFailedLogins) attemptsNumber = 1; + + if (attemptsNumber > maxNumberOfFailedLogins) { + await failed_attempts().updateOne( + { userId: userId }, + { + $set: { + [attemptName]: { + count: attemptsNumber, + createdAt: new Date(), + }, + }, + }, + ); + + return { success: false, attemptsNumber }; + } + + return { success: true, attemptsNumber }; +}; From 0752019d2b52fdaaae94d37ed71444ba2e693e2b Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Tue, 2 Aug 2022 12:38:58 +0200 Subject: [PATCH 2/9] added rate limiters to registration and reset password request --- .../api/src/controllers/auth.ts | 67 ++++++++++--------- .../api/src/model/failed_attempts.ts | 58 ++++++++-------- .../src/util/single-window-rate-limiters.ts | 40 +++++++++++ 3 files changed, 101 insertions(+), 64 deletions(-) create mode 100644 verification/curator-service/api/src/util/single-window-rate-limiters.ts diff --git a/verification/curator-service/api/src/controllers/auth.ts b/verification/curator-service/api/src/controllers/auth.ts index 392612353..5cba2ebb4 100644 --- a/verification/curator-service/api/src/controllers/auth.ts +++ b/verification/curator-service/api/src/controllers/auth.ts @@ -23,25 +23,16 @@ import * as crypto from 'crypto'; import EmailClient from '../clients/email-client'; import { ObjectId } from 'mongodb'; import { baseURL, welcomeEmail } from '../util/instance-details'; -import rateLimit from 'express-rate-limit'; import { failed_attempts, setupFailedAttempts, handleCheckFailedAttempts, } from '../model/failed_attempts'; - -const loginLimiter = rateLimit({ - windowMs: 60 * 60 * 1000, // 60 minutes - max: 4, // Limit each IP to 4 requests per `window` (here, per 60 minutes) - standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers - legacyHeaders: false, // Disable the `X-RateLimit-*` headers - handler: function (req, res /*next*/) { - return res.status(429).json({ - message: - 'You sent too many requests. Please wait a while then try again', - }); - }, -}); +import { + loginLimiter, + registerLimiter, + resetPasswordLimiter, +} from '../util/single-window-rate-limiters'; // Global variable for newsletter acceptance let isNewsletterAccepted: boolean; @@ -211,6 +202,7 @@ export class AuthController { this.router.post( '/signup', + registerLimiter, (req: Request, res: Response, next: NextFunction): void => { passport.authenticate( 'register', @@ -451,6 +443,7 @@ export class AuthController { */ this.router.post( '/request-password-reset', + resetPasswordLimiter, async (req: Request, res: Response): Promise> => { const email = req.body.email as string; @@ -466,6 +459,28 @@ export class AuthController { return res.sendStatus(200); } + const success = await handleCheckFailedAttempts( + user._id, + 'resetPasswordAttempt', + ); + + if (!success) + return res.status(429).json({ + message: 'Too Many Attempts try it one hour later', + }); + + await failed_attempts().updateOne( + { userId: user._id }, + { + $set: { + resetPasswordAttempt: { + count: 0, + createdAt: new Date(), + }, + }, + }, + ); + // Check if user is a Gmail user and send appropriate email message in that case // 42 googleID was set for non Google accounts in the past just to pass mongoose validation // so this check has to be made @@ -765,11 +780,10 @@ export class AuthController { }); } - const { success, attemptsNumber } = - await handleCheckFailedAttempts( - user._id, - 'loginAttempt', - ); + const success = await handleCheckFailedAttempts( + user._id, + 'loginAttempt', + ); if (!success) return done( @@ -786,23 +800,10 @@ export class AuthController { password, ); - if (!isValidPassword) { - await failed_attempts().updateOne( - { userId: user._id }, - { - $set: { - loginAttempt: { - count: attemptsNumber, - createdAt: new Date(), - }, - }, - }, - ); - + if (!isValidPassword) return done(null, false, { message: 'Wrong username or password', }); - } await failed_attempts().updateOne( { userId: user._id }, diff --git a/verification/curator-service/api/src/model/failed_attempts.ts b/verification/curator-service/api/src/model/failed_attempts.ts index 15f9742d2..203a598d9 100644 --- a/verification/curator-service/api/src/model/failed_attempts.ts +++ b/verification/curator-service/api/src/model/failed_attempts.ts @@ -1,15 +1,18 @@ import { Collection, ObjectId } from 'mongodb'; import db from './database'; -/* - * This is a minimal case schema to support some source-related behaviour. - * The full schema for cases is in the data service. - */ - -const maxNumberOfFailedLogins = 8; -const timeWindowForFailedLogins = 60; //min +const numberTimeLimiters = { + loginAttempt: { + maxNumberOfFailedLogins: 8, + timeWindowForFailedLogins: 60, //min + }, + resetPasswordAttempt: { + maxNumberOfFailedLogins: 6, + timeWindowForFailedLogins: 30, + }, +}; -type attemptName = 'loginAttempt' | 'registerAttempt' | 'resetPasswordAttempt'; +type attemptName = 'loginAttempt' | 'resetPasswordAttempt'; export type IfailedAttempts = { _id: ObjectId; @@ -18,10 +21,6 @@ export type IfailedAttempts = { count: number; createdAt: Date; }; - registerAttempt: { - count: number; - createdAt: Date; - }; resetPasswordAttempt: { count: number; createdAt: Date; @@ -39,10 +38,6 @@ export const setupFailedAttempts = async (userId: ObjectId) => { count: 0, createdAt: new Date(), }, - registerAttempt: { - count: 0, - createdAt: new Date(), - }, resetPasswordAttempt: { count: 0, createdAt: new Date(), @@ -66,23 +61,24 @@ export const handleCheckFailedAttempts = async ( 1000, ) / 60; - if (diffTimeMin >= timeWindowForFailedLogins) attemptsNumber = 1; + if ( + diffTimeMin >= numberTimeLimiters[attemptName].timeWindowForFailedLogins + ) + attemptsNumber = 1; - if (attemptsNumber > maxNumberOfFailedLogins) { - await failed_attempts().updateOne( - { userId: userId }, - { - $set: { - [attemptName]: { - count: attemptsNumber, - createdAt: new Date(), - }, + await failed_attempts().updateOne( + { userId: userId }, + { + $set: { + [attemptName]: { + count: attemptsNumber, + createdAt: new Date(), }, }, - ); - - return { success: false, attemptsNumber }; - } + }, + ); - return { success: true, attemptsNumber }; + return ( + attemptsNumber < numberTimeLimiters[attemptName].maxNumberOfFailedLogins + ); }; diff --git a/verification/curator-service/api/src/util/single-window-rate-limiters.ts b/verification/curator-service/api/src/util/single-window-rate-limiters.ts new file mode 100644 index 000000000..2d0e4f799 --- /dev/null +++ b/verification/curator-service/api/src/util/single-window-rate-limiters.ts @@ -0,0 +1,40 @@ +import rateLimit from 'express-rate-limit'; + +export const loginLimiter = rateLimit({ + windowMs: 20 * 60 * 1000, // 20 minutes + max: 4, // Limit each IP to 4 requests per `window` (here, per 20 minutes) + standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers + legacyHeaders: false, // Disable the `X-RateLimit-*` headers + handler: function (req, res /*next*/) { + return res.status(429).json({ + message: + 'You sent too many requests. Please wait a while then try again', + }); + }, +}); + +export const registerLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, // 60 minutes + max: 4, + standardHeaders: true, + legacyHeaders: false, + handler: function (req, res) { + return res.status(429).json({ + message: + 'You sent too many requests. Please wait a while then try again', + }); + }, +}); + +export const resetPasswordLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, // 60 minutes + max: 3, + standardHeaders: true, + legacyHeaders: false, + handler: function (req, res) { + return res.status(429).json({ + message: + 'You sent too many requests. Please wait a while then try again', + }); + }, +}); From deedd9ab3c43e017dfc4a287298a9b3a83cc2873 Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Wed, 3 Aug 2022 12:36:18 +0200 Subject: [PATCH 3/9] password strength bar --- .../curator-service/ui/package-lock.json | 31 +++++++++++++++++++ verification/curator-service/ui/package.json | 1 + .../components/landing-page/SignUpForm.tsx | 14 +++++++++ 3 files changed, 46 insertions(+) diff --git a/verification/curator-service/ui/package-lock.json b/verification/curator-service/ui/package-lock.json index 1fe517893..b9f235c81 100644 --- a/verification/curator-service/ui/package-lock.json +++ b/verification/curator-service/ui/package-lock.json @@ -36,6 +36,7 @@ "react-gtm-module": "^2.0.11", "react-helmet": "^6.1.0", "react-highlight-words": "^0.17.0", + "react-password-strength-bar": "^0.4.1", "react-redux": "^7.2.5", "react-router-dom": "^5.3.0", "react-router-last-location": "^2.0.1", @@ -23245,6 +23246,18 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==" }, + "node_modules/react-password-strength-bar": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/react-password-strength-bar/-/react-password-strength-bar-0.4.1.tgz", + "integrity": "sha512-2NvYz4IUU8k7KDZgsXKoJWreKCZLKGaqF5QhIVhc09OsPBFXFMh0BeghNkBIRkaxLeI7/xjivknDCYfluBCXKA==", + "dependencies": { + "zxcvbn": "4.4.2" + }, + "peerDependencies": { + "react": ">=16.8.6", + "react-dom": ">=16.8.6" + } + }, "node_modules/react-redux": { "version": "7.2.6", "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-7.2.6.tgz", @@ -29433,6 +29446,11 @@ "engines": { "node": ">=10" } + }, + "node_modules/zxcvbn": { + "version": "4.4.2", + "resolved": "https://registry.npmjs.org/zxcvbn/-/zxcvbn-4.4.2.tgz", + "integrity": "sha512-Bq0B+ixT/DMyG8kgX2xWcI5jUvCwqrMxSFam7m0lAf78nf04hv6lNCsyLYdyYTrCVMqNDY/206K7eExYCeSyUQ==" } }, "dependencies": { @@ -47414,6 +47432,14 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==" }, + "react-password-strength-bar": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/react-password-strength-bar/-/react-password-strength-bar-0.4.1.tgz", + "integrity": "sha512-2NvYz4IUU8k7KDZgsXKoJWreKCZLKGaqF5QhIVhc09OsPBFXFMh0BeghNkBIRkaxLeI7/xjivknDCYfluBCXKA==", + "requires": { + "zxcvbn": "4.4.2" + } + }, "react-redux": { "version": "7.2.6", "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-7.2.6.tgz", @@ -52288,6 +52314,11 @@ "property-expr": "^2.0.4", "toposort": "^2.0.2" } + }, + "zxcvbn": { + "version": "4.4.2", + "resolved": "https://registry.npmjs.org/zxcvbn/-/zxcvbn-4.4.2.tgz", + "integrity": "sha512-Bq0B+ixT/DMyG8kgX2xWcI5jUvCwqrMxSFam7m0lAf78nf04hv6lNCsyLYdyYTrCVMqNDY/206K7eExYCeSyUQ==" } } } diff --git a/verification/curator-service/ui/package.json b/verification/curator-service/ui/package.json index b5925371b..aab00f574 100644 --- a/verification/curator-service/ui/package.json +++ b/verification/curator-service/ui/package.json @@ -31,6 +31,7 @@ "react-gtm-module": "^2.0.11", "react-helmet": "^6.1.0", "react-highlight-words": "^0.17.0", + "react-password-strength-bar": "^0.4.1", "react-redux": "^7.2.5", "react-router-dom": "^5.3.0", "react-router-last-location": "^2.0.1", diff --git a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx index 2aa161b39..42448f4d5 100644 --- a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx @@ -21,6 +21,7 @@ import Checkbox from '@material-ui/core/Checkbox'; import Typography from '@material-ui/core/Typography'; import GoogleButton from 'react-google-button'; import { sendCustomGtmEvent } from '../util/helperFunctions'; +import PasswordStrengthBar from 'react-password-strength-bar'; const useStyles = makeStyles((theme: Theme) => ({ checkboxRoot: { @@ -99,6 +100,7 @@ export default function SignUpForm({ const dispatch = useAppDispatch(); const [passwordVisible, setPasswordVisible] = useState(false); + const [passwordStrenght, setPasswordStrenght] = useState(0); const [passwordConfirmationVisible, setPasswordConfirmationVisible] = useState(false); @@ -124,6 +126,9 @@ export default function SignUpForm({ }, ), password: Yup.string() + .test('password-strong-enough', 'Password too weak', () => { + return passwordStrenght > 2; + }) .matches(lowercaseRegex, 'One lowercase required') .matches(uppercaseRegex, 'One uppercase required') .matches(numericRegex, 'One number required') @@ -191,6 +196,7 @@ export default function SignUpForm({ helperText={ formik.touched.email && formik.errors.email } + style={{ marginBottom: 17 }} /> + { + setPasswordStrenght(score); + }} + /> {formik.touched.password && formik.errors.password} From 59438c57b0808de81648f4cc89cb526241e5f5ef Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Mon, 8 Aug 2022 07:09:32 +0200 Subject: [PATCH 4/9] added request limiters to 3 methods of changing password, password strength bar, test for login and registration --- .../api/src/controllers/auth.ts | 110 ++++++++++++------ .../api/src/model/failed_attempts.ts | 79 ++++++++++--- .../src/util/single-window-rate-limiters.ts | 31 ++++- .../components/LandingPage.spec.ts | 47 ++++++++ .../components/ProfileTest.spec.ts | 18 ++- .../integration/components/UsersTest.spec.ts | 12 +- .../ui/src/components/Profile.tsx | 13 +++ .../landing-page/ChangePasswordForm.tsx | 13 +++ .../components/landing-page/SignUpForm.tsx | 6 +- 9 files changed, 261 insertions(+), 68 deletions(-) diff --git a/verification/curator-service/api/src/controllers/auth.ts b/verification/curator-service/api/src/controllers/auth.ts index 5cba2ebb4..7a8e95265 100644 --- a/verification/curator-service/api/src/controllers/auth.ts +++ b/verification/curator-service/api/src/controllers/auth.ts @@ -24,14 +24,17 @@ import EmailClient from '../clients/email-client'; import { ObjectId } from 'mongodb'; import { baseURL, welcomeEmail } from '../util/instance-details'; import { - failed_attempts, setupFailedAttempts, handleCheckFailedAttempts, + attemptName, + updateFailedAttempts, } from '../model/failed_attempts'; import { loginLimiter, registerLimiter, resetPasswordLimiter, + forgotPasswordLimiter, + resetPasswordWithTokenLimiter, } from '../util/single-window-rate-limiters'; // Global variable for newsletter acceptance @@ -392,6 +395,7 @@ export class AuthController { */ this.router.post( '/change-password', + resetPasswordLimiter, mustBeAuthenticated, async (req: Request, res: Response) => { const oldPassword = req.body.oldPassword as string; @@ -411,6 +415,24 @@ export class AuthController { return res.sendStatus(403); } + const { success, attemptsNumber } = + await handleCheckFailedAttempts( + currentUser._id, + attemptName.ResetPassword, + ); + + if (!success) + return res.status(429).json({ + message: + 'Too many failed login attempts, please try again later', + }); + + updateFailedAttempts( + currentUser._id, + attemptName.ResetPassword, + attemptsNumber, + ); + const isValidPassword = await isUserPasswordValid( currentUser, oldPassword, @@ -443,14 +465,14 @@ export class AuthController { */ this.router.post( '/request-password-reset', - resetPasswordLimiter, + forgotPasswordLimiter, async (req: Request, res: Response): Promise> => { const email = req.body.email as string; try { // Check if user with this email address exists const userPromise = await users() - .find({ email: email }) + .find({ email }) .collation({ locale: 'en_US', strength: 2 }) .toArray(); @@ -459,26 +481,23 @@ export class AuthController { return res.sendStatus(200); } - const success = await handleCheckFailedAttempts( - user._id, - 'resetPasswordAttempt', - ); + const { success, attemptsNumber } = + await handleCheckFailedAttempts( + user._id, + attemptName.ForgotPassword, + ); - if (!success) + if (!success) { return res.status(429).json({ - message: 'Too Many Attempts try it one hour later', + message: + 'You sent too many requests. Please wait a while then try again', }); + } - await failed_attempts().updateOne( - { userId: user._id }, - { - $set: { - resetPasswordAttempt: { - count: 0, - createdAt: new Date(), - }, - }, - }, + updateFailedAttempts( + user._id, + attemptName.ResetPassword, + attemptsNumber, ); // Check if user is a Gmail user and send appropriate email message in that case @@ -549,6 +568,7 @@ export class AuthController { */ this.router.post( '/reset-password', + resetPasswordWithTokenLimiter, async (req: Request, res: Response): Promise => { const userId = req.body.userId; const token = req.body.token as string; @@ -561,6 +581,24 @@ export class AuthController { throw new Error('Invalid user id'); } + const { success, attemptsNumber } = + await handleCheckFailedAttempts( + userId, + attemptName.ResetPasswordWithToken, + ); + + if (!success) + res.status(429).json({ + message: + 'Too many failed login attempts, please try again later', + }); + + updateFailedAttempts( + userId, + attemptName.ForgotPassword, + attemptsNumber, + ); + // Check if token exists const passwordResetToken = await tokens().findOne({ userId, @@ -780,10 +818,11 @@ export class AuthController { }); } - const success = await handleCheckFailedAttempts( - user._id, - 'loginAttempt', - ); + const { success, attemptsNumber } = + await handleCheckFailedAttempts( + user._id, + attemptName.Login, + ); if (!success) return done( @@ -791,7 +830,7 @@ export class AuthController { { timeout: true }, { message: - 'Too Many Attempts try it one hour later', + 'Too many failed login attempts, please try again later', }, ); @@ -800,22 +839,19 @@ export class AuthController { password, ); - if (!isValidPassword) + if (!isValidPassword) { + updateFailedAttempts( + user._id, + attemptName.Login, + attemptsNumber, + ); + return done(null, false, { message: 'Wrong username or password', }); + } - await failed_attempts().updateOne( - { userId: user._id }, - { - $set: { - loginAttempt: { - count: 0, - createdAt: new Date(), - }, - }, - }, - ); + updateFailedAttempts(user._id, attemptName.Login, 0); done(null, user); } catch (error) { done(error); @@ -950,7 +986,7 @@ export class AuthController { ); } const userPromise = await users() - .find({ email: email }) + .find({ email }) .collation({ locale: 'en_US', strength: 2 }) .toArray(); diff --git a/verification/curator-service/api/src/model/failed_attempts.ts b/verification/curator-service/api/src/model/failed_attempts.ts index 203a598d9..e5ba650a3 100644 --- a/verification/curator-service/api/src/model/failed_attempts.ts +++ b/verification/curator-service/api/src/model/failed_attempts.ts @@ -7,14 +7,27 @@ const numberTimeLimiters = { timeWindowForFailedLogins: 60, //min }, resetPasswordAttempt: { - maxNumberOfFailedLogins: 6, - timeWindowForFailedLogins: 30, + maxNumberOfFailedLogins: 8, + timeWindowForFailedLogins: 60, + }, + forgotPasswordAttempt: { + maxNumberOfFailedLogins: 8, + timeWindowForFailedLogins: 60, + }, + resetPasswordWithTokenAttempt: { + maxNumberOfFailedLogins: 8, + timeWindowForFailedLogins: 60, }, }; -type attemptName = 'loginAttempt' | 'resetPasswordAttempt'; +export enum attemptName { + Login = 'loginAttempt', + ResetPassword = 'resetPasswordAttempt', + ForgotPassword = 'forgotPasswordAttempt', + ResetPasswordWithToken = 'resetPasswordWithTokenAttempt', +} -export type IfailedAttempts = { +export interface IFailedAttempts { _id: ObjectId; userId: ObjectId; loginAttempt: { @@ -25,13 +38,21 @@ export type IfailedAttempts = { count: number; createdAt: Date; }; -}; + forgotPasswordAttempt: { + count: number; + createdAt: Date; + }; + resetPasswordWithTokenAttempt: { + count: number; + createdAt: Date; + }; +} -export const failed_attempts = () => - db().collection('failed_attempts') as Collection; +export const failedAttempts = () => + db().collection('failedAttempts') as Collection; export const setupFailedAttempts = async (userId: ObjectId) => { - await failed_attempts().insertOne({ + await failedAttempts().insertOne({ _id: new ObjectId(), userId: userId, loginAttempt: { @@ -42,6 +63,14 @@ export const setupFailedAttempts = async (userId: ObjectId) => { count: 0, createdAt: new Date(), }, + forgotPasswordAttempt: { + count: 0, + createdAt: new Date(), + }, + resetPasswordWithTokenAttempt: { + count: 0, + createdAt: new Date(), + }, }); }; @@ -49,9 +78,16 @@ export const handleCheckFailedAttempts = async ( userId: ObjectId, attemptName: attemptName, ) => { - const attempts = (await failed_attempts().findOne({ - userId: userId, - })) as IfailedAttempts; + let attempts = await failedAttempts().findOne({ + userId, + }); + + if (!attempts) { + setupFailedAttempts(userId); + attempts = (await failedAttempts().findOne({ + userId, + })) as IFailedAttempts; + } let attemptsNumber = attempts[attemptName].count + 1; @@ -66,8 +102,21 @@ export const handleCheckFailedAttempts = async ( ) attemptsNumber = 1; - await failed_attempts().updateOne( - { userId: userId }, + return { + success: + attemptsNumber < + numberTimeLimiters[attemptName].maxNumberOfFailedLogins, + attemptsNumber, + }; +}; + +export const updateFailedAttempts = async ( + userId: ObjectId, + attemptName: attemptName, + attemptsNumber: number, +) => { + await failedAttempts().updateOne( + { userId }, { $set: { [attemptName]: { @@ -77,8 +126,4 @@ export const handleCheckFailedAttempts = async ( }, }, ); - - return ( - attemptsNumber < numberTimeLimiters[attemptName].maxNumberOfFailedLogins - ); }; diff --git a/verification/curator-service/api/src/util/single-window-rate-limiters.ts b/verification/curator-service/api/src/util/single-window-rate-limiters.ts index 2d0e4f799..eaf8d671d 100644 --- a/verification/curator-service/api/src/util/single-window-rate-limiters.ts +++ b/verification/curator-service/api/src/util/single-window-rate-limiters.ts @@ -7,8 +7,7 @@ export const loginLimiter = rateLimit({ legacyHeaders: false, // Disable the `X-RateLimit-*` headers handler: function (req, res /*next*/) { return res.status(429).json({ - message: - 'You sent too many requests. Please wait a while then try again', + message: 'Too many failed login attempts, please try again later', }); }, }); @@ -28,7 +27,33 @@ export const registerLimiter = rateLimit({ export const resetPasswordLimiter = rateLimit({ windowMs: 60 * 60 * 1000, // 60 minutes - max: 3, + max: 4, + standardHeaders: true, + legacyHeaders: false, + handler: function (req, res) { + return res.status(429).json({ + message: + 'You sent too many requests. Please wait a while then try again', + }); + }, +}); + +export const forgotPasswordLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, // 60 minutes + max: 4, + standardHeaders: true, + legacyHeaders: false, + handler: function (req, res) { + return res.status(429).json({ + message: + 'You sent too many requests. Please wait a while then try again', + }); + }, +}); + +export const resetPasswordWithTokenLimiter = rateLimit({ + windowMs: 60 * 60 * 1000, // 60 minutes + max: 4, standardHeaders: true, legacyHeaders: false, handler: function (req, res) { diff --git a/verification/curator-service/ui/cypress/integration/components/LandingPage.spec.ts b/verification/curator-service/ui/cypress/integration/components/LandingPage.spec.ts index 49563f3c1..266f06689 100644 --- a/verification/curator-service/ui/cypress/integration/components/LandingPage.spec.ts +++ b/verification/curator-service/ui/cypress/integration/components/LandingPage.spec.ts @@ -53,6 +53,18 @@ describe('LandingPage', function () { cy.get('#password').type('tT$5'); cy.get('button[data-testid="sign-up-button"]').click(); cy.contains(/Minimum 8 characters required/i); + + //check score 1 strength of password + cy.get('#password').focus().clear(); + cy.get('#password').type('Tt1ttttt'); + cy.get('button[data-testid="sign-up-button"]').click(); + cy.contains(/Password too weak/i); + + //check score 2 strength of password + cy.get('#password').focus().clear(); + cy.get('#password').type('tT$5aaaaa'); + cy.get('button[data-testid="sign-up-button"]').click(); + cy.contains(/Password too weak/i); }); it('Validates emails', function () { @@ -123,6 +135,18 @@ describe('LandingPage', function () { cy.get('#password').type('tT$5'); cy.get('button[data-testid="change-password-button"]').click(); cy.contains('Minimum 8 characters required!'); + + //check score 1 strength of password + cy.get('#password').focus().clear(); + cy.get('#password').type('Tt1ttttt'); + cy.get('button[data-testid="change-password-button"]').click(); + cy.contains('Password too weak'); + + //check score 2 strength of password + cy.get('#password').focus().clear(); + cy.get('#password').type('tT$5aaaaa'); + cy.get('button[data-testid="change-password-button"]').click(); + cy.contains('Password too weak'); }); it('Homepage with logged out user', function () { @@ -186,4 +210,27 @@ describe('LandingPage', function () { cy.contains('Manage users').should('not.exist'); cy.contains('Terms of use'); }); + + it('Limit number of requests to register and login ', function () { + cy.visit('/'); + cy.get('#email').type('test@example.com'); + cy.get('#confirmEmail').type('test@example.com'); + cy.get('#password').type('tT$5aaaaak'); + cy.get('#passwordConfirmation').type('tT$5aaaaak'); + cy.get('#isAgreementChecked').check(); + for (let i = 0; i < 5; i++) { + cy.get('button[data-testid="sign-up-button"]').click(); + } + cy.contains( + /You sent too many requests. Please wait a while then try again/i, + ); + + cy.contains('Sign in!').click(); + cy.get('#email').type('test@example.com'); + cy.get('#password').type('test'); + for (let i = 0; i < 5; i++) { + cy.get('button[data-testid="sign-in-button"]').click(); + } + cy.contains(/Too many failed login attempts, please try again later/i); + }); }); diff --git a/verification/curator-service/ui/cypress/integration/components/ProfileTest.spec.ts b/verification/curator-service/ui/cypress/integration/components/ProfileTest.spec.ts index 54e3f93ba..ae163958a 100644 --- a/verification/curator-service/ui/cypress/integration/components/ProfileTest.spec.ts +++ b/verification/curator-service/ui/cypress/integration/components/ProfileTest.spec.ts @@ -20,10 +20,12 @@ describe('Profile', function () { email: 'alice@test.com', roles: ['curator'], }); - cy.visit('/') + cy.visit('/'); cy.visit('/profile'); - cy.get('[data-testid="change-your-password-title"]').should('not.exist'); + cy.get('[data-testid="change-your-password-title"]').should( + 'not.exist', + ); }); it('Checks if the change pass form validation works well', function () { @@ -52,6 +54,18 @@ describe('Profile', function () { cy.get('#password').type('tT$5'); cy.get('button[data-testid="change-password-button"]').click(); cy.contains('Minimum 8 characters required!'); + + //check score 1 strength of password + cy.get('#password').focus().clear(); + cy.get('#password').type('Tt1ttttt'); + cy.get('button[data-testid="change-password-button"]').click(); + cy.contains('Password too weak'); + + //check score 2 strength of password + cy.get('#password').focus().clear(); + cy.get('#password').type('tT$5aaaaa'); + cy.get('button[data-testid="change-password-button"]').click(); + cy.contains('Password too weak'); }); it('Checks if the validates the repeated password', function () { diff --git a/verification/curator-service/ui/cypress/integration/components/UsersTest.spec.ts b/verification/curator-service/ui/cypress/integration/components/UsersTest.spec.ts index 15fe4cbe5..30c67b697 100644 --- a/verification/curator-service/ui/cypress/integration/components/UsersTest.spec.ts +++ b/verification/curator-service/ui/cypress/integration/components/UsersTest.spec.ts @@ -11,7 +11,7 @@ describe('Manage users page', function () { roles: ['curator'], }); cy.login({ name: 'Alice', email: 'alice@test.com', roles: ['admin'] }); - cy.visit('/') + cy.visit('/'); cy.visit('/users'); cy.contains('Alice'); @@ -33,8 +33,8 @@ describe('Manage users page', function () { roles: ['curator'], }); cy.login({ name: 'Alice', email: 'alice@test.com', roles: ['admin'] }); - cy.visit('/') - cy.visit('/users') + cy.visit('/'); + cy.visit('/users'); cy.contains('Bob'); cy.get('div[data-testid="Bob-select-roles"]').contains('curator'); cy.get('div[data-testid="Bob-select-roles"]') @@ -60,7 +60,7 @@ describe('Manage users page', function () { .should('not.exist'); // Roles are maintained on refresh - cy.visit('/') + cy.visit('/'); cy.visit('/users'); cy.get('div[data-testid="Bob-select-roles"]').contains('admin'); cy.get('div[data-testid="Bob-select-roles"]') @@ -70,7 +70,7 @@ describe('Manage users page', function () { it('Updated roles propagate to other pages', function () { cy.login({ name: 'Alice', email: 'alice@test.com', roles: ['admin'] }); - cy.visit('/') + cy.visit('/'); cy.visit('/users'); // Select new role @@ -85,7 +85,7 @@ describe('Manage users page', function () { cy.contains('Line list'); // Profile page is updated - cy.visit('/') + cy.visit('/'); cy.visit('/profile'); cy.contains('admin'); cy.contains('curator'); diff --git a/verification/curator-service/ui/src/components/Profile.tsx b/verification/curator-service/ui/src/components/Profile.tsx index 9000c5700..677ed8242 100644 --- a/verification/curator-service/ui/src/components/Profile.tsx +++ b/verification/curator-service/ui/src/components/Profile.tsx @@ -33,6 +33,7 @@ import Alert from '@material-ui/lab/Alert'; import LinearProgress from '@material-ui/core/LinearProgress'; import { SnackbarAlert } from './SnackbarAlert'; import Helmet from 'react-helmet'; +import PasswordStrengthBar from 'react-password-strength-bar'; const styles = makeStyles((theme: Theme) => ({ root: { @@ -119,6 +120,7 @@ export function ChangePasswordFormInProfile(): JSX.Element { const classes = useStyles(); const dispatch = useAppDispatch(); + const [passwordStrenght, setPasswordStrenght] = useState(0); const [oldPasswordVisible, setOldPasswordVisible] = useState(false); const [passwordVisible, setPasswordVisible] = useState(false); const [passwordConfirmationVisible, setPasswordConfirmationVisible] = @@ -138,6 +140,9 @@ export function ChangePasswordFormInProfile(): JSX.Element { .matches(uppercaseRegex, 'one uppercase required!') .matches(numericRegex, 'one number required!') .min(8, 'Minimum 8 characters required!') + .test('password-strong-enough', 'Password too weak', () => { + return passwordStrenght > 2; + }) .test( 'passwords-different', "New password can't be the same as old password", @@ -280,6 +285,14 @@ export function ChangePasswordFormInProfile(): JSX.Element { } label="New password" /> + { + setPasswordStrenght(score); + }} + /> {formik.touched.password && formik.errors.password} diff --git a/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx b/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx index a3a5fbc35..cffef50ce 100644 --- a/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx @@ -18,6 +18,7 @@ import Visibility from '@material-ui/icons/Visibility'; import VisibilityOff from '@material-ui/icons/VisibilityOff'; import Button from '@material-ui/core/Button'; import Typography from '@material-ui/core/Typography'; +import PasswordStrengthBar from 'react-password-strength-bar'; const useStyles = makeStyles((theme: Theme) => ({ checkboxRoot: { @@ -88,6 +89,7 @@ export default function ChangePasswordForm({ const dispatch = useAppDispatch(); const history = useHistory(); + const [passwordStrenght, setPasswordStrenght] = useState(0); const passwordReset = useAppSelector(selectPasswordReset); const [passwordVisible, setPasswordVisible] = useState(false); const [passwordConfirmationVisible, setPasswordConfirmationVisible] = @@ -122,6 +124,9 @@ export default function ChangePasswordForm({ .matches(uppercaseRegex, 'one uppercase required!') .matches(numericRegex, 'one number required!') .min(8, 'Minimum 8 characters required!') + .test('password-strong-enough', 'Password too weak', () => { + return passwordStrenght > 2; + }) .required('Required!'), passwordConfirmation: Yup.string().test( 'passwords-match', @@ -203,6 +208,14 @@ export default function ChangePasswordForm({ } label="Password" /> + { + setPasswordStrenght(score); + }} + /> {formik.touched.password && formik.errors.password} diff --git a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx index 42448f4d5..12be68ad6 100644 --- a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx @@ -126,13 +126,13 @@ export default function SignUpForm({ }, ), password: Yup.string() - .test('password-strong-enough', 'Password too weak', () => { - return passwordStrenght > 2; - }) .matches(lowercaseRegex, 'One lowercase required') .matches(uppercaseRegex, 'One uppercase required') .matches(numericRegex, 'One number required') .min(8, 'Minimum 8 characters required') + .test('password-strong-enough', 'Password too weak', () => { + return passwordStrenght > 2; + }) .required('This field is required'), passwordConfirmation: Yup.string().test( 'passwords-match', From e752ce369845a840ade117f5a8825436e611a222 Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Tue, 9 Aug 2022 06:14:24 +0200 Subject: [PATCH 5/9] changes after review, added limiters to 3 types of reseting password --- .../api/src/controllers/auth.ts | 36 ++++++++++++------- .../src/util/single-window-rate-limiters.ts | 2 +- verification/curator-service/ui/.gitignore | 1 + 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/verification/curator-service/api/src/controllers/auth.ts b/verification/curator-service/api/src/controllers/auth.ts index 7a8e95265..a6655aae8 100644 --- a/verification/curator-service/api/src/controllers/auth.ts +++ b/verification/curator-service/api/src/controllers/auth.ts @@ -427,23 +427,29 @@ export class AuthController { 'Too many failed login attempts, please try again later', }); - updateFailedAttempts( - currentUser._id, - attemptName.ResetPassword, - attemptsNumber, - ); - const isValidPassword = await isUserPasswordValid( currentUser, oldPassword, ); if (!isValidPassword) { + updateFailedAttempts( + currentUser._id, + attemptName.ResetPassword, + attemptsNumber, + ); + return res .status(403) .json({ message: 'Old password is incorrect' }); } + updateFailedAttempts( + currentUser._id, + attemptName.ResetPassword, + 0, + ); + const hashedPassword = await bcrypt.hash(newPassword, 10); await users().updateOne(userQuery, { $set: { @@ -593,17 +599,16 @@ export class AuthController { 'Too many failed login attempts, please try again later', }); - updateFailedAttempts( - userId, - attemptName.ForgotPassword, - attemptsNumber, - ); - // Check if token exists const passwordResetToken = await tokens().findOne({ userId, }); if (!passwordResetToken) { + updateFailedAttempts( + userId, + attemptName.ForgotPassword, + attemptsNumber, + ); throw new Error( 'Invalid or expired password reset token', ); @@ -615,6 +620,11 @@ export class AuthController { passwordResetToken.token, ); if (!isValid) { + updateFailedAttempts( + userId, + attemptName.ForgotPassword, + attemptsNumber, + ); throw new Error( 'Invalid or expired password reset token', ); @@ -641,6 +651,8 @@ export class AuthController { // Send confirmation email to the user const user = result.value as IUser; + updateFailedAttempts(userId, attemptName.ForgotPassword, 0); + await this.emailClient.send( [user.email], 'Password Change Confirmation', diff --git a/verification/curator-service/api/src/util/single-window-rate-limiters.ts b/verification/curator-service/api/src/util/single-window-rate-limiters.ts index eaf8d671d..5ca7d32cc 100644 --- a/verification/curator-service/api/src/util/single-window-rate-limiters.ts +++ b/verification/curator-service/api/src/util/single-window-rate-limiters.ts @@ -26,7 +26,7 @@ export const registerLimiter = rateLimit({ }); export const resetPasswordLimiter = rateLimit({ - windowMs: 60 * 60 * 1000, // 60 minutes + windowMs: 30 * 60 * 1000, // 30 minutes max: 4, standardHeaders: true, legacyHeaders: false, diff --git a/verification/curator-service/ui/.gitignore b/verification/curator-service/ui/.gitignore index a7e808add..4997e7efe 100644 --- a/verification/curator-service/ui/.gitignore +++ b/verification/curator-service/ui/.gitignore @@ -1,6 +1,7 @@ # Env files .env .env.staging +.env.development # Don't include CSVs, except for Cypress fixtures. *.csv From 2f4a4a8d9700e782098c33dbc9f8edfad16d1d19 Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Wed, 10 Aug 2022 14:16:25 +0200 Subject: [PATCH 6/9] CR fixes --- .../api/src/controllers/auth.ts | 26 +++++++++---------- .../api/src/model/failed_attempts.ts | 6 ++--- .../landing-page/ChangePasswordForm.tsx | 8 +++--- .../components/landing-page/SignUpForm.tsx | 8 +++--- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/verification/curator-service/api/src/controllers/auth.ts b/verification/curator-service/api/src/controllers/auth.ts index a6655aae8..8b60210f7 100644 --- a/verification/curator-service/api/src/controllers/auth.ts +++ b/verification/curator-service/api/src/controllers/auth.ts @@ -26,7 +26,7 @@ import { baseURL, welcomeEmail } from '../util/instance-details'; import { setupFailedAttempts, handleCheckFailedAttempts, - attemptName, + AttemptName, updateFailedAttempts, } from '../model/failed_attempts'; import { @@ -418,7 +418,7 @@ export class AuthController { const { success, attemptsNumber } = await handleCheckFailedAttempts( currentUser._id, - attemptName.ResetPassword, + AttemptName.ResetPassword, ); if (!success) @@ -435,7 +435,7 @@ export class AuthController { if (!isValidPassword) { updateFailedAttempts( currentUser._id, - attemptName.ResetPassword, + AttemptName.ResetPassword, attemptsNumber, ); @@ -446,7 +446,7 @@ export class AuthController { updateFailedAttempts( currentUser._id, - attemptName.ResetPassword, + AttemptName.ResetPassword, 0, ); @@ -490,7 +490,7 @@ export class AuthController { const { success, attemptsNumber } = await handleCheckFailedAttempts( user._id, - attemptName.ForgotPassword, + AttemptName.ForgotPassword, ); if (!success) { @@ -502,7 +502,7 @@ export class AuthController { updateFailedAttempts( user._id, - attemptName.ResetPassword, + AttemptName.ResetPassword, attemptsNumber, ); @@ -590,7 +590,7 @@ export class AuthController { const { success, attemptsNumber } = await handleCheckFailedAttempts( userId, - attemptName.ResetPasswordWithToken, + AttemptName.ResetPasswordWithToken, ); if (!success) @@ -606,7 +606,7 @@ export class AuthController { if (!passwordResetToken) { updateFailedAttempts( userId, - attemptName.ForgotPassword, + AttemptName.ForgotPassword, attemptsNumber, ); throw new Error( @@ -622,7 +622,7 @@ export class AuthController { if (!isValid) { updateFailedAttempts( userId, - attemptName.ForgotPassword, + AttemptName.ForgotPassword, attemptsNumber, ); throw new Error( @@ -651,7 +651,7 @@ export class AuthController { // Send confirmation email to the user const user = result.value as IUser; - updateFailedAttempts(userId, attemptName.ForgotPassword, 0); + updateFailedAttempts(userId, AttemptName.ForgotPassword, 0); await this.emailClient.send( [user.email], @@ -833,7 +833,7 @@ export class AuthController { const { success, attemptsNumber } = await handleCheckFailedAttempts( user._id, - attemptName.Login, + AttemptName.Login, ); if (!success) @@ -854,7 +854,7 @@ export class AuthController { if (!isValidPassword) { updateFailedAttempts( user._id, - attemptName.Login, + AttemptName.Login, attemptsNumber, ); @@ -863,7 +863,7 @@ export class AuthController { }); } - updateFailedAttempts(user._id, attemptName.Login, 0); + updateFailedAttempts(user._id, AttemptName.Login, 0); done(null, user); } catch (error) { done(error); diff --git a/verification/curator-service/api/src/model/failed_attempts.ts b/verification/curator-service/api/src/model/failed_attempts.ts index e5ba650a3..f16c550fc 100644 --- a/verification/curator-service/api/src/model/failed_attempts.ts +++ b/verification/curator-service/api/src/model/failed_attempts.ts @@ -20,7 +20,7 @@ const numberTimeLimiters = { }, }; -export enum attemptName { +export enum AttemptName { Login = 'loginAttempt', ResetPassword = 'resetPasswordAttempt', ForgotPassword = 'forgotPasswordAttempt', @@ -76,7 +76,7 @@ export const setupFailedAttempts = async (userId: ObjectId) => { export const handleCheckFailedAttempts = async ( userId: ObjectId, - attemptName: attemptName, + attemptName: AttemptName, ) => { let attempts = await failedAttempts().findOne({ userId, @@ -112,7 +112,7 @@ export const handleCheckFailedAttempts = async ( export const updateFailedAttempts = async ( userId: ObjectId, - attemptName: attemptName, + attemptName: AttemptName, attemptsNumber: number, ) => { await failedAttempts().updateOne( diff --git a/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx b/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx index cffef50ce..d3c780235 100644 --- a/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx @@ -89,7 +89,7 @@ export default function ChangePasswordForm({ const dispatch = useAppDispatch(); const history = useHistory(); - const [passwordStrenght, setPasswordStrenght] = useState(0); + const [passwordStrength, setPasswordStrength] = useState(0); const passwordReset = useAppSelector(selectPasswordReset); const [passwordVisible, setPasswordVisible] = useState(false); const [passwordConfirmationVisible, setPasswordConfirmationVisible] = @@ -125,7 +125,7 @@ export default function ChangePasswordForm({ .matches(numericRegex, 'one number required!') .min(8, 'Minimum 8 characters required!') .test('password-strong-enough', 'Password too weak', () => { - return passwordStrenght > 2; + return passwordStrength > 2; }) .required('Required!'), passwordConfirmation: Yup.string().test( @@ -212,8 +212,8 @@ export default function ChangePasswordForm({ password={formik.values.password} scoreWords={[]} shortScoreWord="" - onChangeScore={(score) => { - setPasswordStrenght(score); + onChangeScore={(score: number) => { + setPasswordStrength(score); }} /> diff --git a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx index 12be68ad6..2dc57c165 100644 --- a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx @@ -100,7 +100,7 @@ export default function SignUpForm({ const dispatch = useAppDispatch(); const [passwordVisible, setPasswordVisible] = useState(false); - const [passwordStrenght, setPasswordStrenght] = useState(0); + const [passwordStrength, setPasswordStrength] = useState(0); const [passwordConfirmationVisible, setPasswordConfirmationVisible] = useState(false); @@ -131,7 +131,7 @@ export default function SignUpForm({ .matches(numericRegex, 'One number required') .min(8, 'Minimum 8 characters required') .test('password-strong-enough', 'Password too weak', () => { - return passwordStrenght > 2; + return passwordStrength > 2; }) .required('This field is required'), passwordConfirmation: Yup.string().test( @@ -264,8 +264,8 @@ export default function SignUpForm({ password={formik.values.password} scoreWords={[]} shortScoreWord="" - onChangeScore={(score) => { - setPasswordStrenght(score); + onChangeScore={(score: number) => { + setPasswordStrength(score); }} /> From 7ad3b4732c59cd7911b7639c10ec35d5424a3b9a Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Wed, 10 Aug 2022 14:58:25 +0200 Subject: [PATCH 7/9] test fix --- .../curator-service/ui/src/components/Profile.test.tsx | 8 ++++---- .../curator-service/ui/src/components/Profile.tsx | 8 ++++---- .../ui/src/components/landing-page/ChangePasswordForm.tsx | 4 ++-- .../ui/src/components/landing-page/SignUpForm.tsx | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/verification/curator-service/ui/src/components/Profile.test.tsx b/verification/curator-service/ui/src/components/Profile.test.tsx index 4df2f312e..d052e3b04 100644 --- a/verification/curator-service/ui/src/components/Profile.test.tsx +++ b/verification/curator-service/ui/src/components/Profile.test.tsx @@ -193,10 +193,10 @@ describe('', () => { render(, { initialState: noUserInfoState }); userEvent.type(screen.getByLabelText('Old Password'), '1234567'); - userEvent.type(screen.getByLabelText('New password'), 'asdD?234'); + userEvent.type(screen.getByLabelText('New password'), 'asdD?234df'); userEvent.type( screen.getByLabelText('Repeat new password'), - 'asdD?234', + 'asdD?234df', ); userEvent.click( @@ -225,10 +225,10 @@ describe('', () => { render(, { initialState: noUserInfoState }); userEvent.type(screen.getByLabelText('Old Password'), '1234567'); - userEvent.type(screen.getByLabelText('New password'), 'asdD?234'); + userEvent.type(screen.getByLabelText('New password'), 'asdD?234df'); userEvent.type( screen.getByLabelText('Repeat new password'), - 'asdD?234', + 'asdD?234df', ); userEvent.click( diff --git a/verification/curator-service/ui/src/components/Profile.tsx b/verification/curator-service/ui/src/components/Profile.tsx index 677ed8242..82e3293cd 100644 --- a/verification/curator-service/ui/src/components/Profile.tsx +++ b/verification/curator-service/ui/src/components/Profile.tsx @@ -140,9 +140,7 @@ export function ChangePasswordFormInProfile(): JSX.Element { .matches(uppercaseRegex, 'one uppercase required!') .matches(numericRegex, 'one number required!') .min(8, 'Minimum 8 characters required!') - .test('password-strong-enough', 'Password too weak', () => { - return passwordStrenght > 2; - }) + .required('Required!') .test( 'passwords-different', "New password can't be the same as old password", @@ -150,7 +148,9 @@ export function ChangePasswordFormInProfile(): JSX.Element { return this.parent.oldPassword !== value; }, ) - .required('Required!'), + .test('password-strong-enough', 'Password too weak', () => { + return passwordStrenght > 2; + }), passwordConfirmation: Yup.string().test( 'passwords-match', 'Passwords must match', diff --git a/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx b/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx index d3c780235..3fe555844 100644 --- a/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/ChangePasswordForm.tsx @@ -124,10 +124,10 @@ export default function ChangePasswordForm({ .matches(uppercaseRegex, 'one uppercase required!') .matches(numericRegex, 'one number required!') .min(8, 'Minimum 8 characters required!') + .required('Required!') .test('password-strong-enough', 'Password too weak', () => { return passwordStrength > 2; - }) - .required('Required!'), + }), passwordConfirmation: Yup.string().test( 'passwords-match', 'Passwords must match', diff --git a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx index 2dc57c165..2a9d269f5 100644 --- a/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx +++ b/verification/curator-service/ui/src/components/landing-page/SignUpForm.tsx @@ -130,10 +130,10 @@ export default function SignUpForm({ .matches(uppercaseRegex, 'One uppercase required') .matches(numericRegex, 'One number required') .min(8, 'Minimum 8 characters required') + .required('This field is required') .test('password-strong-enough', 'Password too weak', () => { return passwordStrength > 2; - }) - .required('This field is required'), + }), passwordConfirmation: Yup.string().test( 'passwords-match', 'Passwords must match', From 5a7ad5b1b7fbf42e3d76649c2b37a7dcccee5bcb Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Thu, 11 Aug 2022 06:25:34 +0200 Subject: [PATCH 8/9] correcting spelling --- .../curator-service/ui/src/components/Profile.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/verification/curator-service/ui/src/components/Profile.tsx b/verification/curator-service/ui/src/components/Profile.tsx index 82e3293cd..4d72a1736 100644 --- a/verification/curator-service/ui/src/components/Profile.tsx +++ b/verification/curator-service/ui/src/components/Profile.tsx @@ -120,7 +120,7 @@ export function ChangePasswordFormInProfile(): JSX.Element { const classes = useStyles(); const dispatch = useAppDispatch(); - const [passwordStrenght, setPasswordStrenght] = useState(0); + const [passwordStrength, setPasswordStrength] = useState(0); const [oldPasswordVisible, setOldPasswordVisible] = useState(false); const [passwordVisible, setPasswordVisible] = useState(false); const [passwordConfirmationVisible, setPasswordConfirmationVisible] = @@ -149,7 +149,7 @@ export function ChangePasswordFormInProfile(): JSX.Element { }, ) .test('password-strong-enough', 'Password too weak', () => { - return passwordStrenght > 2; + return passwordStrength > 2; }), passwordConfirmation: Yup.string().test( 'passwords-match', @@ -289,8 +289,8 @@ export function ChangePasswordFormInProfile(): JSX.Element { password={formik.values.password} scoreWords={[]} shortScoreWord="" - onChangeScore={(score) => { - setPasswordStrenght(score); + onChangeScore={(score: number) => { + setPasswordStrength(score); }} /> From 8519e5778a15e2d967caf86e974ef487a2b7e32c Mon Sep 17 00:00:00 2001 From: Oskar Kocjan Date: Fri, 12 Aug 2022 13:28:50 +0200 Subject: [PATCH 9/9] CR changes --- .../curator-service/api/src/controllers/auth.ts | 12 ++++++++---- .../curator-service/api/src/model/failed_attempts.ts | 11 ++++++----- .../api/src/util/single-window-rate-limiters.ts | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/verification/curator-service/api/src/controllers/auth.ts b/verification/curator-service/api/src/controllers/auth.ts index 8b60210f7..528222493 100644 --- a/verification/curator-service/api/src/controllers/auth.ts +++ b/verification/curator-service/api/src/controllers/auth.ts @@ -502,7 +502,7 @@ export class AuthController { updateFailedAttempts( user._id, - AttemptName.ResetPassword, + AttemptName.ForgotPassword, attemptsNumber, ); @@ -606,7 +606,7 @@ export class AuthController { if (!passwordResetToken) { updateFailedAttempts( userId, - AttemptName.ForgotPassword, + AttemptName.ResetPasswordWithToken, attemptsNumber, ); throw new Error( @@ -622,7 +622,7 @@ export class AuthController { if (!isValid) { updateFailedAttempts( userId, - AttemptName.ForgotPassword, + AttemptName.ResetPasswordWithToken, attemptsNumber, ); throw new Error( @@ -651,7 +651,11 @@ export class AuthController { // Send confirmation email to the user const user = result.value as IUser; - updateFailedAttempts(userId, AttemptName.ForgotPassword, 0); + updateFailedAttempts( + userId, + AttemptName.ResetPasswordWithToken, + 0, + ); await this.emailClient.send( [user.email], diff --git a/verification/curator-service/api/src/model/failed_attempts.ts b/verification/curator-service/api/src/model/failed_attempts.ts index f16c550fc..eb51851df 100644 --- a/verification/curator-service/api/src/model/failed_attempts.ts +++ b/verification/curator-service/api/src/model/failed_attempts.ts @@ -4,19 +4,19 @@ import db from './database'; const numberTimeLimiters = { loginAttempt: { maxNumberOfFailedLogins: 8, - timeWindowForFailedLogins: 60, //min + timeWindowForFailedLoginsMinutes: 60, }, resetPasswordAttempt: { maxNumberOfFailedLogins: 8, - timeWindowForFailedLogins: 60, + timeWindowForFailedLoginsMinutes: 60, }, forgotPasswordAttempt: { maxNumberOfFailedLogins: 8, - timeWindowForFailedLogins: 60, + timeWindowForFailedLoginsMinutes: 60, }, resetPasswordWithTokenAttempt: { maxNumberOfFailedLogins: 8, - timeWindowForFailedLogins: 60, + timeWindowForFailedLoginsMinutes: 60, }, }; @@ -98,7 +98,8 @@ export const handleCheckFailedAttempts = async ( ) / 60; if ( - diffTimeMin >= numberTimeLimiters[attemptName].timeWindowForFailedLogins + diffTimeMin >= + numberTimeLimiters[attemptName].timeWindowForFailedLoginsMinutes ) attemptsNumber = 1; diff --git a/verification/curator-service/api/src/util/single-window-rate-limiters.ts b/verification/curator-service/api/src/util/single-window-rate-limiters.ts index 5ca7d32cc..f125a9744 100644 --- a/verification/curator-service/api/src/util/single-window-rate-limiters.ts +++ b/verification/curator-service/api/src/util/single-window-rate-limiters.ts @@ -1,7 +1,7 @@ import rateLimit from 'express-rate-limit'; export const loginLimiter = rateLimit({ - windowMs: 20 * 60 * 1000, // 20 minutes + windowMs: 60 * 60 * 1000, // 60 minutes max: 4, // Limit each IP to 4 requests per `window` (here, per 20 minutes) standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers legacyHeaders: false, // Disable the `X-RateLimit-*` headers @@ -26,7 +26,7 @@ export const registerLimiter = rateLimit({ }); export const resetPasswordLimiter = rateLimit({ - windowMs: 30 * 60 * 1000, // 30 minutes + windowMs: 60 * 60 * 1000, // 60 minutes max: 4, standardHeaders: true, legacyHeaders: false,