From 55dff1ca9a82fe884b58690dd551af077b2a424c Mon Sep 17 00:00:00 2001 From: abichan99911111 Date: Fri, 27 Sep 2024 08:36:56 +0000 Subject: [PATCH 1/8] reject requests with invalid email format --- apps/app/src/server/routes/apiv3/forgot-password.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index cb8a8c84071..8ed909df7c6 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -62,11 +62,16 @@ module.exports = (crowi) => { } router.post('/', checkPassportStrategyMiddleware, addActivity, async(req, res) => { + const validEmailRegexp = new RegExp(/^[\w+\-.]+@[a-z\d\-.]+\.[a-z]+$/, 'i'); const { email } = req.body; const locale = configManager.getConfig('crowi', 'app:globalLang'); const appUrl = appService.getSiteUrl(); try { + if (!validEmailRegexp.test(email.toString())) { + throw new Error('invalid email format.'); + } + const user = await User.findOne({ email }); // when the user is not found or active From dbf266e252f88c1429a1b1ea4df8c8a629884a86 Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 06:34:39 +0000 Subject: [PATCH 2/8] fix lint --- apps/app/src/server/routes/forgot-password.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/app/src/server/routes/forgot-password.ts b/apps/app/src/server/routes/forgot-password.ts index 9dfcc5aec80..925d58f06b3 100644 --- a/apps/app/src/server/routes/forgot-password.ts +++ b/apps/app/src/server/routes/forgot-password.ts @@ -1,4 +1,4 @@ -import { +import type { NextFunction, Request, Response, } from 'express'; import createError from 'http-errors'; @@ -6,7 +6,7 @@ import createError from 'http-errors'; import { forgotPasswordErrorCode } from '~/interfaces/errors/forgot-password'; import loggerFactory from '~/utils/logger'; -import { IPasswordResetOrder } from '../models/password-reset-order'; +import type { IPasswordResetOrder } from '../models/password-reset-order'; const logger = loggerFactory('growi:routes:forgot-password'); From 504e26817a13221ee7305df173ea5ecdfaf5c1b3 Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 06:36:30 +0000 Subject: [PATCH 3/8] use filterXSS --- apps/app/src/server/routes/apiv3/forgot-password.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index 8ed909df7c6..0b78606ac17 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -1,6 +1,7 @@ import { ErrorV3 } from '@growi/core/dist/models'; import { serializeUserSecurely } from '@growi/core/dist/models/serializers'; import { format, subSeconds } from 'date-fns'; +import { FilterXSS } from 'xss'; import { SupportedAction } from '~/interfaces/activity'; import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity'; @@ -18,6 +19,7 @@ const logger = loggerFactory('growi:routes:apiv3:forgotPassword'); // eslint-dis const express = require('express'); const { body } = require('express-validator'); +const filterXss = new FilterXSS(); const router = express.Router(); @@ -62,15 +64,11 @@ module.exports = (crowi) => { } router.post('/', checkPassportStrategyMiddleware, addActivity, async(req, res) => { - const validEmailRegexp = new RegExp(/^[\w+\-.]+@[a-z\d\-.]+\.[a-z]+$/, 'i'); - const { email } = req.body; + const email = filterXss.process(req.body.email); const locale = configManager.getConfig('crowi', 'app:globalLang'); const appUrl = appService.getSiteUrl(); try { - if (!validEmailRegexp.test(email.toString())) { - throw new Error('invalid email format.'); - } const user = await User.findOne({ email }); @@ -102,7 +100,7 @@ module.exports = (crowi) => { // eslint-disable-next-line max-len router.put('/', checkPassportStrategyMiddleware, injectResetOrderByTokenMiddleware, validator.password, apiV3FormValidator, addActivity, async(req, res) => { const { passwordResetOrder } = req; - const { email } = passwordResetOrder; + const email = filterXss.process(passwordResetOrder.email); const grobalLang = configManager.getConfig('crowi', 'app:globalLang'); const i18n = grobalLang || req.language; const { newPassword } = req.body; From 8e5cc826b1e5561b6deb07f67c644fec20c27bcd Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 08:00:16 +0000 Subject: [PATCH 4/8] use validator --- .../src/server/routes/apiv3/forgot-password.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index 0b78606ac17..e8458f100e0 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -19,8 +19,6 @@ const logger = loggerFactory('growi:routes:apiv3:forgotPassword'); // eslint-dis const express = require('express'); const { body } = require('express-validator'); -const filterXss = new FilterXSS(); - const router = express.Router(); module.exports = (crowi) => { @@ -45,6 +43,13 @@ module.exports = (crowi) => { return (value === req.body.newPassword); }), ], + email: [ + body('email') + .isEmail() + .withMessage('message.Email format is invalid') + .exists() + .withMessage('message.Email field is required'), + ], }; const checkPassportStrategyMiddleware = checkForgotPasswordEnabledMiddlewareFactory(crowi, true); @@ -63,8 +68,8 @@ module.exports = (crowi) => { }); } - router.post('/', checkPassportStrategyMiddleware, addActivity, async(req, res) => { - const email = filterXss.process(req.body.email); + router.post('/', checkPassportStrategyMiddleware, validator.email, addActivity, async(req, res) => { + const { email } = req.body; const locale = configManager.getConfig('crowi', 'app:globalLang'); const appUrl = appService.getSiteUrl(); @@ -100,7 +105,7 @@ module.exports = (crowi) => { // eslint-disable-next-line max-len router.put('/', checkPassportStrategyMiddleware, injectResetOrderByTokenMiddleware, validator.password, apiV3FormValidator, addActivity, async(req, res) => { const { passwordResetOrder } = req; - const email = filterXss.process(passwordResetOrder.email); + const { email } = passwordResetOrder; const grobalLang = configManager.getConfig('crowi', 'app:globalLang'); const i18n = grobalLang || req.language; const { newPassword } = req.body; From 8a4ee115d15636faa66260862d37edd7bc74124f Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 08:40:15 +0000 Subject: [PATCH 5/8] add validator forgot-password --- apps/app/src/server/routes/apiv3/forgot-password.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index e8458f100e0..fbefafd432b 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -1,7 +1,6 @@ import { ErrorV3 } from '@growi/core/dist/models'; import { serializeUserSecurely } from '@growi/core/dist/models/serializers'; import { format, subSeconds } from 'date-fns'; -import { FilterXSS } from 'xss'; import { SupportedAction } from '~/interfaces/activity'; import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity'; @@ -17,7 +16,7 @@ import { checkForgotPasswordEnabledMiddlewareFactory } from '../forgot-password' const logger = loggerFactory('growi:routes:apiv3:forgotPassword'); // eslint-disable-line no-unused-vars const express = require('express'); -const { body } = require('express-validator'); +const { body, validationResult } = require('express-validator'); const router = express.Router(); @@ -46,8 +45,9 @@ module.exports = (crowi) => { email: [ body('email') .isEmail() + .escape() .withMessage('message.Email format is invalid') - .exists() + .notEmpty() .withMessage('message.Email field is required'), ], }; @@ -69,12 +69,16 @@ module.exports = (crowi) => { } router.post('/', checkPassportStrategyMiddleware, validator.email, addActivity, async(req, res) => { - const { email } = req.body; const locale = configManager.getConfig('crowi', 'app:globalLang'); const appUrl = appService.getSiteUrl(); try { + const error = validationResult(req); + if (!error.isEmpty()) { + throw Error('invalid email format'); + } + const email = req.query.email; const user = await User.findOne({ email }); // when the user is not found or active From 3c20a2a519f7290d7e509717b81bfbc3ad48e757 Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 08:44:11 +0000 Subject: [PATCH 6/8] add validatorResult for new password --- apps/app/src/server/routes/apiv3/forgot-password.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index fbefafd432b..e1d7c538108 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -112,7 +112,7 @@ module.exports = (crowi) => { const { email } = passwordResetOrder; const grobalLang = configManager.getConfig('crowi', 'app:globalLang'); const i18n = grobalLang || req.language; - const { newPassword } = req.body; + const user = await User.findOne({ email }); @@ -122,6 +122,11 @@ module.exports = (crowi) => { } try { + const error = validationResult(req); + if (!error.isEmpty) { + throw Error('invalid password format'); + } + const { newPassword } = req.body; const userData = await user.updatePassword(newPassword); const serializedUserData = serializeUserSecurely(userData); passwordResetOrder.revokeOneTimeToken(); From de75a67d1b2ca079479cc08becce750e8cacf70f Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 09:17:31 +0000 Subject: [PATCH 7/8] use apiV3FormValidator --- .../src/server/routes/apiv3/forgot-password.js | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index e1d7c538108..99b3a2ad5a3 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -68,17 +68,12 @@ module.exports = (crowi) => { }); } - router.post('/', checkPassportStrategyMiddleware, validator.email, addActivity, async(req, res) => { + router.post('/', checkPassportStrategyMiddleware, validator.email, apiV3FormValidator, addActivity, async(req, res) => { + const { email } = req.query; const locale = configManager.getConfig('crowi', 'app:globalLang'); const appUrl = appService.getSiteUrl(); try { - - const error = validationResult(req); - if (!error.isEmpty()) { - throw Error('invalid email format'); - } - const email = req.query.email; const user = await User.findOne({ email }); // when the user is not found or active @@ -113,6 +108,7 @@ module.exports = (crowi) => { const grobalLang = configManager.getConfig('crowi', 'app:globalLang'); const i18n = grobalLang || req.language; + const { newPassword } = req.body; const user = await User.findOne({ email }); @@ -122,11 +118,6 @@ module.exports = (crowi) => { } try { - const error = validationResult(req); - if (!error.isEmpty) { - throw Error('invalid password format'); - } - const { newPassword } = req.body; const userData = await user.updatePassword(newPassword); const serializedUserData = serializeUserSecurely(userData); passwordResetOrder.revokeOneTimeToken(); From 6a3ce0d9a36b79af9d1c147568a0cb3492647fdc Mon Sep 17 00:00:00 2001 From: reiji-h Date: Thu, 3 Oct 2024 09:20:45 +0000 Subject: [PATCH 8/8] clean code --- apps/app/src/server/middlewares/apiv3-form-validator.ts | 2 +- apps/app/src/server/routes/apiv3/forgot-password.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/app/src/server/middlewares/apiv3-form-validator.ts b/apps/app/src/server/middlewares/apiv3-form-validator.ts index 6d3c2c92eda..8ea216daf22 100644 --- a/apps/app/src/server/middlewares/apiv3-form-validator.ts +++ b/apps/app/src/server/middlewares/apiv3-form-validator.ts @@ -1,5 +1,5 @@ import { ErrorV3 } from '@growi/core/dist/models'; -import { NextFunction, Request, Response } from 'express'; +import type { NextFunction, Request, Response } from 'express'; import loggerFactory from '~/utils/logger'; diff --git a/apps/app/src/server/routes/apiv3/forgot-password.js b/apps/app/src/server/routes/apiv3/forgot-password.js index 99b3a2ad5a3..8bcea069a86 100644 --- a/apps/app/src/server/routes/apiv3/forgot-password.js +++ b/apps/app/src/server/routes/apiv3/forgot-password.js @@ -16,7 +16,8 @@ import { checkForgotPasswordEnabledMiddlewareFactory } from '../forgot-password' const logger = loggerFactory('growi:routes:apiv3:forgotPassword'); // eslint-disable-line no-unused-vars const express = require('express'); -const { body, validationResult } = require('express-validator'); +const { body } = require('express-validator'); + const router = express.Router(); @@ -69,7 +70,7 @@ module.exports = (crowi) => { } router.post('/', checkPassportStrategyMiddleware, validator.email, apiV3FormValidator, addActivity, async(req, res) => { - const { email } = req.query; + const { email } = req.body; const locale = configManager.getConfig('crowi', 'app:globalLang'); const appUrl = appService.getSiteUrl(); @@ -107,7 +108,6 @@ module.exports = (crowi) => { const { email } = passwordResetOrder; const grobalLang = configManager.getConfig('crowi', 'app:globalLang'); const i18n = grobalLang || req.language; - const { newPassword } = req.body; const user = await User.findOne({ email });