From f65b72c5b42f7996c227bd1a8efec4eaf1d12a22 Mon Sep 17 00:00:00 2001 From: wancat Date: Wed, 1 Nov 2023 22:11:17 +0800 Subject: [PATCH] feat: add rate limit for authentication * improve error handling in frontend --- app/src/utils/auth.js | 54 ++++++++++++++++++++--- app/src/utils/errors.js | 27 ++++++++++++ app/src/views/LoginPage.vue | 18 +++++++- app/src/views/RegisterPage.vue | 30 +++++++------ server/src/api/routes/auth.js | 21 +++++++-- server/src/api/routes/sessions.js | 2 +- server/src/index.js | 2 +- server/src/middleware/globalRateLimit.js | 6 --- server/src/middleware/rateLimiter.js | 19 ++++++++ server/src/middleware/sessionRateLimit.js | 9 ---- 10 files changed, 146 insertions(+), 42 deletions(-) create mode 100644 app/src/utils/errors.js delete mode 100644 server/src/middleware/globalRateLimit.js create mode 100644 server/src/middleware/rateLimiter.js delete mode 100644 server/src/middleware/sessionRateLimit.js diff --git a/app/src/utils/auth.js b/app/src/utils/auth.js index 8e6a478..d91e2b8 100644 --- a/app/src/utils/auth.js +++ b/app/src/utils/auth.js @@ -1,5 +1,11 @@ import api from './api' +import { + TooManyRequestsError, + UnauthorizedError, + ValidationError, +} from './errors' + export async function isLoggedIn() { const res = await api.get('/auth/check-session') const { loggedIn } = res.data @@ -12,18 +18,52 @@ export const login = async (username, password) => { return true } catch (err) { if (err.response?.status === 401) { - return false + throw new UnauthorizedError(err.response.data.message) + } + if (err.response?.status === 429) { + throw new TooManyRequestsError(err.response.data.message) + } + if (err.response?.status === 400) { + throw new ValidationError(err.response.data.message) } throw err } } -export const register = (username, email, password) => { - return api.post('/auth/register', { - username, - password, - email, - }) +export const register = async (username, email, password) => { + try { + await api.post('/auth/register', { + username, + password, + email, + }) + return true + } catch (err) { + const status = err.response?.status + if (status === 429) { + throw new TooManyRequestsError(err.response.data.message) + } + if (status === 401) { + throw new UnauthorizedError(err.response.data.message) + } + if (status === 400) { + const { errors } = err.response.data + if (errors?.length > 0) { + const { path, msg } = errors[0] + switch (path) { + case 'username': + throw new ValidationError('Username is already taken') + case 'email': + throw new ValidationError('Email is already taken') + case 'password': + throw new ValidationError('Password is too weak') + default: + throw new ValidationError(msg) + } + } + } + throw err + } } export const logout = () => { diff --git a/app/src/utils/errors.js b/app/src/utils/errors.js new file mode 100644 index 0000000..2fe0cbb --- /dev/null +++ b/app/src/utils/errors.js @@ -0,0 +1,27 @@ +/* eslint-disable max-classes-per-file */ +export class TooManyRequestsError extends Error { + constructor(message) { + super(message) + this.name = 'TooManyRequestsError' + } +} + +export class ValidationError extends Error { + constructor(message) { + super(message) + this.name = 'ValidationError' + } +} + +export class UnauthorizedError extends Error { + constructor(message) { + super(message) + this.name = 'UnauthorizedError' + } +} + +export default { + TooManyRequestsError, + ValidationError, + UnauthorizedError, +} diff --git a/app/src/views/LoginPage.vue b/app/src/views/LoginPage.vue index 35e354c..0c12075 100644 --- a/app/src/views/LoginPage.vue +++ b/app/src/views/LoginPage.vue @@ -2,14 +2,30 @@ import AuthForm from '../components/AuthForm.vue' import Background from '../components/DynamicBackground.vue' +import { + TooManyRequestsError, + UnauthorizedError, + ValidationError, +} from '../utils/errors' import { login } from '../utils/auth' const handleLogin = async ({ username, password, success, error }) => { try { const isSuccess = await login(username, password) if (isSuccess) success() - else error('Wrong username or password.') } catch (err) { + if (err instanceof TooManyRequestsError) { + error('Too many requests. Try again in 2 minutes.') + return + } + if (err instanceof UnauthorizedError) { + error('Wrong username or password.') + return + } + if (err instanceof ValidationError) { + error('Invalid username or password.') + return + } console.error(err) error('Something went wrong.') } diff --git a/app/src/views/RegisterPage.vue b/app/src/views/RegisterPage.vue index f8cb967..ab8e7a9 100644 --- a/app/src/views/RegisterPage.vue +++ b/app/src/views/RegisterPage.vue @@ -7,25 +7,29 @@ import Background from '../components/DynamicBackground.vue' import api from '../utils/api' import { register } from '../utils/auth' +import { + UnauthorizedError, + TooManyRequestsError, + ValidationError, +} from '../utils/errors' function handleRegister({ username, email, password, success, error }) { register(username, email, password) .then(success) .catch((err) => { - if (err.response?.status === 409) { - // Handle username or email already exists error here - if (err.response.data.type === 'username') { - error('Username already exists.') - } else if (err.response.data.type === 'email') { - error('Email already exists.') - } else { - error('Something went wrong.') - } - } else if (err.response?.status === 400) { - error('Invalid username or email.') - } else { - console.error(err) + if (err instanceof TooManyRequestsError) { + error('Too many requests. Try again in 2 minutes.') + return } + if (err instanceof UnauthorizedError) { + error('Invalid username or password.') + return + } + if (err instanceof ValidationError) { + error(err.message) + return + } + console.error(err) }) } diff --git a/server/src/api/routes/auth.js b/server/src/api/routes/auth.js index eb42353..8bea9a9 100644 --- a/server/src/api/routes/auth.js +++ b/server/src/api/routes/auth.js @@ -16,11 +16,13 @@ import { registerFromSessionValidators, registerValidators, } from '../validators/authValidator.js' +import { authenticateRateLimit } from '../../middleware/rateLimiter.js' const router = Router() router.post( '/login', + authenticateRateLimit, loginValidators, passport.authenticate('local', { failureMessage: true }), (req, res) => { @@ -28,9 +30,9 @@ router.post( } ) -router.post('/register', registerValidators, register) +router.post('/register', authenticateRateLimit, registerValidators, register) -router.post('/logout', logout) +router.post('/logout', authenticateRateLimit, logout) router.get('/check-username', checkUsernameValidators, (req, res) => res.json({ available: true }) @@ -43,16 +45,27 @@ router.get('/available-methods', (req, res) => { if (enabledMethods.google) { router.get('/google', passport.authenticate('google')) - router.get('/google/callback', passport.authenticate('google'), socialLogin) + router.get( + '/google/callback', + authenticateRateLimit, + passport.authenticate('google'), + socialLogin + ) } if (enabledMethods.github) { router.get('/github', passport.authenticate('github')) - router.get('/github/callback', passport.authenticate('github'), socialLogin) + router.get( + '/github/callback', + authenticateRateLimit, + passport.authenticate('github'), + socialLogin + ) } router.post( '/register-from-session', + authenticateRateLimit, registerFromSessionValidators, registerFromSession ) diff --git a/server/src/api/routes/sessions.js b/server/src/api/routes/sessions.js index d6b5f9d..86961d5 100644 --- a/server/src/api/routes/sessions.js +++ b/server/src/api/routes/sessions.js @@ -1,6 +1,6 @@ import { Router } from 'express' import * as sessionController from '../controllers/sessionController.js' -import sessionRateLimit from '../../middleware/sessionRateLimit.js' +import { sessionRateLimit } from '../../middleware/rateLimiter.js' const router = Router() diff --git a/server/src/index.js b/server/src/index.js index 8960767..67f4566 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -12,7 +12,7 @@ import apiRouter from './api/routes/index.js' import loadAndUpdateQuests from './utils/quest.js' import config from './config.js' import errorHandler from './middleware/error.js' -import globalRateLimit from './middleware/globalRateLimit.js' +import { globalRateLimit } from './middleware/rateLimiter.js' import sessionMiddleware from './middleware/session.js' import expiryRemovalScheduler from './containers/expiryChecker.js' diff --git a/server/src/middleware/globalRateLimit.js b/server/src/middleware/globalRateLimit.js deleted file mode 100644 index 1ca314e..0000000 --- a/server/src/middleware/globalRateLimit.js +++ /dev/null @@ -1,6 +0,0 @@ -import { rateLimit } from 'express-rate-limit' - -export default rateLimit({ - windowMs: 60 * 1000, // 1 minute - max: 100, -}) diff --git a/server/src/middleware/rateLimiter.js b/server/src/middleware/rateLimiter.js new file mode 100644 index 0000000..85b1f5d --- /dev/null +++ b/server/src/middleware/rateLimiter.js @@ -0,0 +1,19 @@ +import rateLimit from 'express-rate-limit' + +export const globalRateLimit = rateLimit({ + windowMs: 60 * 1000, // 1 minute + max: 100, +}) + +export const authenticateRateLimit = rateLimit({ + windowMs: 2 * 60 * 1000, + max: 5, +}) + +export const sessionRateLimit = rateLimit({ + windowMs: 10 * 1000, + max: 1, + keyGenerator: (req) => { + return req.user ? req.user._id : req.ip + }, +}) diff --git a/server/src/middleware/sessionRateLimit.js b/server/src/middleware/sessionRateLimit.js deleted file mode 100644 index be008e6..0000000 --- a/server/src/middleware/sessionRateLimit.js +++ /dev/null @@ -1,9 +0,0 @@ -import { rateLimit } from 'express-rate-limit' - -export default rateLimit({ - windowMs: 10 * 1000, - max: 1, - keyGenerator: (req) => { - return req.user ? req.user._id : req.ip - }, -})