From 0fb085002beb2291f815c7f4577792047e3fcdd6 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 19:21:00 -0500 Subject: [PATCH 01/24] add redirect if either param is missing - working on flash --- .../views/account/get/confirmPasswordReset.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index eb600c43..147e77ac 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -7,10 +7,23 @@ exports = module.exports = function (req, res) { locals.formData = req.body || {} - const flash = null // Render the view locals.username = req.query.username locals.token = req.query.token - res.render('account/confirmPasswordReset', { flash }) + + if (!locals.username || !locals.token) { + const flash = { + class: 'alert-danger', + messages: [{ msg: 'Invalid reset request.' }], + type: 'Error!' + }; + + return res.redirect('/account/requestPasswordReset'); + } + + else { + const flash = null; + res.render('account/confirmPasswordReset', { flash }) + } } From f639adc8b923d31bdf487716b135fcc134175808 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 19:53:26 -0500 Subject: [PATCH 02/24] refactor to be more funcitonal - flashes are harder than ancicipated --- .../views/account/get/confirmPasswordReset.js | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index 147e77ac..8804e761 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -7,23 +7,22 @@ exports = module.exports = function (req, res) { locals.formData = req.body || {} - // Render the view locals.username = req.query.username locals.token = req.query.token - if (!locals.username || !locals.token) { - const flash = { - class: 'alert-danger', - messages: [{ msg: 'Invalid reset request.' }], - type: 'Error!' - }; - - return res.redirect('/account/requestPasswordReset'); + // Likely too verbose - but can be used as a catch-all for further validations + const valid_params = (token, username) => { + if (!token || !username) { + return false + } else { + return true + } } - else { - const flash = null; - res.render('account/confirmPasswordReset', { flash }) - } + valid_params + ? res.render('account/confirmPasswordReset') + : () => { + res.redirect('/account/requestPasswordReset') + } } From 62dc98a24afdb26b85acf72dc720c4e43830d724 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 19:55:46 -0500 Subject: [PATCH 03/24] add .tool-versions to git ignore - this is used by asdf --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index e75192a6..bb3026ae 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,4 @@ public/js/*.js sessions dist +.tool-versions \ No newline at end of file From 0bcd4c7b97ef39a9ed3099a82e0b1909e47cf7c3 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 20:22:41 -0500 Subject: [PATCH 04/24] less verbose - turnary was being problematic --- .../views/account/get/confirmPasswordReset.js | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index 8804e761..06a0adaf 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -11,18 +11,15 @@ exports = module.exports = function (req, res) { locals.username = req.query.username locals.token = req.query.token - // Likely too verbose - but can be used as a catch-all for further validations - const valid_params = (token, username) => { - if (!token || !username) { - return false - } else { - return true - } - } + const valid_req = (!locals.token || !locals.username) - valid_params - ? res.render('account/confirmPasswordReset') - : () => { - res.redirect('/account/requestPasswordReset') - } + if (valid_req) { + let flash = {} + flash.class = 'alert-danger' + flash.messages = [{ msg: 'Invalid reset link' }] + flash.type = 'Error!' + res.redirect('/account/requestPasswordReset') + } else { + res.render('account/confirmPasswordReset') + } } From 0c1f04be1fe3fd8d58f4de086c6ef93e6c0b302d Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:03:16 -0500 Subject: [PATCH 05/24] flashes work with this - however it does not confirm to other standards --- .../views/account/get/confirmPasswordReset.js | 19 ++++++++++++------- .../views/account/get/requestPasswordReset.js | 9 ++++++++- .../views/account/confirmPasswordReset.pug | 2 +- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index 06a0adaf..bc2ea4dc 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -11,14 +11,19 @@ exports = module.exports = function (req, res) { locals.username = req.query.username locals.token = req.query.token - const valid_req = (!locals.token || !locals.username) + // Ensure token and username are present + const validateParamPresence = (token, username) => { + if (!token || !username) { + return "Invalid request" + } else { + return null + } + } + + const errorMsg = validateParamPresence(req.query.username, req.query.token) - if (valid_req) { - let flash = {} - flash.class = 'alert-danger' - flash.messages = [{ msg: 'Invalid reset link' }] - flash.type = 'Error!' - res.redirect('/account/requestPasswordReset') + if (errorMsg) { + res.redirect('/account/requestPasswordReset?flash=' + errorMsg) } else { res.render('account/confirmPasswordReset') } diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index fc1b9a07..c5cfa2c8 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -12,9 +12,16 @@ exports = module.exports = async function (req, res) { throw new Error('java-api error') } + let flash = {} + if (req.query.flash) { + flash.class = 'alert-danger' + flash.messages = [{ msg: req.query.flash }] + flash.type = 'Error!' + } + res.render('account/requestPasswordReset', { section: 'account', - flash: {}, + flash: flash, steamReset: response.data.steamUrl, formData, recaptchaSiteKey: appConfig.recaptchaKey diff --git a/src/backend/templates/views/account/confirmPasswordReset.pug b/src/backend/templates/views/account/confirmPasswordReset.pug index 7be2e504..0c9d8b34 100644 --- a/src/backend/templates/views/account/confirmPasswordReset.pug +++ b/src/backend/templates/views/account/confirmPasswordReset.pug @@ -14,7 +14,7 @@ block content .row .col-md-offset-3.col-md-6 - form(method='post',action="/account/password/confirmReset?username="+username+"&token="+token,data-toggle="validator") + form(method='post', action="/account/password/confirmReset?username="+username+"&token="+token, data-toggle="validator") +confirm-password .form-actions button(type='submit').btn.btn-default.btn-lg.btn-outro.btn-danger Reset From 90497ebb7fe75fb2f6bed98407c408d872e9432f Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:12:00 -0500 Subject: [PATCH 06/24] found an example of this - working flashes following existing code conventions --- .../views/account/get/confirmPasswordReset.js | 14 ++++++++++++-- .../views/account/get/requestPasswordReset.js | 14 ++++++++++---- .../views/account/requestPasswordReset.pug | 4 ++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index bc2ea4dc..faf09c65 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -1,6 +1,8 @@ exports = module.exports = function (req, res) { const locals = res.locals + const overallRes = res + // locals.section is used to set the currently selected // item in the header navigation. locals.section = 'account' @@ -14,7 +16,7 @@ exports = module.exports = function (req, res) { // Ensure token and username are present const validateParamPresence = (token, username) => { if (!token || !username) { - return "Invalid request" + return 'Invalid request' } else { return null } @@ -23,7 +25,15 @@ exports = module.exports = function (req, res) { const errorMsg = validateParamPresence(req.query.username, req.query.token) if (errorMsg) { - res.redirect('/account/requestPasswordReset?flash=' + errorMsg) + flash = {} + flash.class = 'alert-danger' + flash.messages = [{ msg: errorMsg}] + flash.type = 'Error!' + + const buff = Buffer.from(JSON.stringify(flash)) + const data = buff.toString('base64') + + return overallRes.redirect('/account/requestPasswordReset?flash=' + data) } else { res.render('account/confirmPasswordReset') } diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index c5cfa2c8..d7b4be4b 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -12,11 +12,17 @@ exports = module.exports = async function (req, res) { throw new Error('java-api error') } - let flash = {} if (req.query.flash) { - flash.class = 'alert-danger' - flash.messages = [{ msg: req.query.flash }] - flash.type = 'Error!' + const buff = Buffer.from(req.query.flash, 'base64') + const text = buff.toString('ascii') + + try { + flash = JSON.parse(text) + } catch (e) { + console.error('Parsing error while trying to decode a flash error: ' + text) + console.error(e) + flash = [{ msg: 'Unknown error' }] + } } res.render('account/requestPasswordReset', { diff --git a/src/backend/templates/views/account/requestPasswordReset.pug b/src/backend/templates/views/account/requestPasswordReset.pug index 8b50958b..03fc9624 100644 --- a/src/backend/templates/views/account/requestPasswordReset.pug +++ b/src/backend/templates/views/account/requestPasswordReset.pug @@ -27,11 +27,11 @@ block content label.column12 .g-recaptcha(data-sitekey=recaptchaSiteKey) .form-actions - + button(type='submit').btn.btn-default.btn-lg.btn-outro.btn-danger Reset via email br br - + br br From ce71bd81c753c6a8de6e42bb15151ea6d5bdff9f Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:12:54 -0500 Subject: [PATCH 07/24] safe flash --- src/backend/routes/views/account/get/requestPasswordReset.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index d7b4be4b..e5128b93 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -27,7 +27,7 @@ exports = module.exports = async function (req, res) { res.render('account/requestPasswordReset', { section: 'account', - flash: flash, + flash: flash || {}, steamReset: response.data.steamUrl, formData, recaptchaSiteKey: appConfig.recaptchaKey From 0d33d8ba8b24ce9a456543deda086a70bf13c207 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:19:30 -0500 Subject: [PATCH 08/24] fix flash persistence --- src/backend/routes/views/account/get/requestPasswordReset.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index e5128b93..8bad6de4 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -12,6 +12,7 @@ exports = module.exports = async function (req, res) { throw new Error('java-api error') } + let flash = {} if (req.query.flash) { const buff = Buffer.from(req.query.flash, 'base64') const text = buff.toString('ascii') @@ -27,7 +28,7 @@ exports = module.exports = async function (req, res) { res.render('account/requestPasswordReset', { section: 'account', - flash: flash || {}, + flash: flash, steamReset: response.data.steamUrl, formData, recaptchaSiteKey: appConfig.recaptchaKey From f1ef63c9fc64b41a1c361c75ec8fcb37caa0450b Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:23:12 -0500 Subject: [PATCH 09/24] linting --- .../views/account/get/confirmPasswordReset.js | 26 +++++++++---------- .../views/account/get/requestPasswordReset.js | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index faf09c65..8eee3034 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -15,26 +15,26 @@ exports = module.exports = function (req, res) { // Ensure token and username are present const validateParamPresence = (token, username) => { - if (!token || !username) { - return 'Invalid request' - } else { - return null - } + if (!token || !username) { + return 'Invalid request' + } else { + return null + } } const errorMsg = validateParamPresence(req.query.username, req.query.token) if (errorMsg) { - flash = {} - flash.class = 'alert-danger' - flash.messages = [{ msg: errorMsg}] - flash.type = 'Error!' + const flash = {} + flash.class = 'alert-danger' + flash.messages = [{ msg: errorMsg }] + flash.type = 'Error!' - const buff = Buffer.from(JSON.stringify(flash)) - const data = buff.toString('base64') + const buff = Buffer.from(JSON.stringify(flash)) + const data = buff.toString('base64') - return overallRes.redirect('/account/requestPasswordReset?flash=' + data) + return overallRes.redirect('/account/requestPasswordReset?flash=' + data) } else { - res.render('account/confirmPasswordReset') + res.render('account/confirmPasswordReset') } } diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index 8bad6de4..e918ae96 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -28,7 +28,7 @@ exports = module.exports = async function (req, res) { res.render('account/requestPasswordReset', { section: 'account', - flash: flash, + flash, steamReset: response.data.steamUrl, formData, recaptchaSiteKey: appConfig.recaptchaKey From 64fe9fed87899b45c0fa30db6dd69787cfa2a8f1 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:27:59 -0500 Subject: [PATCH 10/24] update unit tests --- tests/integration/accountRouter.test.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/integration/accountRouter.test.js b/tests/integration/accountRouter.test.js index 784f067e..29ba7c9e 100644 --- a/tests/integration/accountRouter.test.js +++ b/tests/integration/accountRouter.test.js @@ -12,7 +12,6 @@ beforeEach(async () => { describe('Account Routes', function () { const publicUrls = [ '/account/requestPasswordReset', - '/account/password/confirmReset', '/account/register', '/account/activate' ] @@ -22,6 +21,18 @@ describe('Account Routes', function () { expect(res.statusCode).toBe(200) }) + test('responds with OK to provided parameters', async () => { + const response = await testSession.get('/account/password/confirmReset?username=turbo2&token=XXXXX') + expect(response.statusCode).toBe(200) + expect(response.headers.location).toBe('/account/requestPasswordReset') + }) + + test('redirect to reset request page if missing parameters', async () => { + const response = await testSession.get('/account/password/confirmReset') + expect(response.statusCode).toBe(200) + expect(response.headers.location).toBe('/account/requestPasswordReset') + }) + test('redirect old pw-reset routes', async () => { const response = await testSession.get('/account/password/reset') expect(response.statusCode).toBe(302) From 6e186cc91722596e9203aa1ea76afad5f3f466fd Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:28:42 -0500 Subject: [PATCH 11/24] lint again - sorry for wasting minutes my bad --- tests/integration/accountRouter.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/accountRouter.test.js b/tests/integration/accountRouter.test.js index 29ba7c9e..90730aa7 100644 --- a/tests/integration/accountRouter.test.js +++ b/tests/integration/accountRouter.test.js @@ -22,15 +22,15 @@ describe('Account Routes', function () { }) test('responds with OK to provided parameters', async () => { - const response = await testSession.get('/account/password/confirmReset?username=turbo2&token=XXXXX') - expect(response.statusCode).toBe(200) - expect(response.headers.location).toBe('/account/requestPasswordReset') + const response = await testSession.get('/account/password/confirmReset?username=turbo2&token=XXXXX') + expect(response.statusCode).toBe(200) + expect(response.headers.location).toBe('/account/requestPasswordReset') }) test('redirect to reset request page if missing parameters', async () => { - const response = await testSession.get('/account/password/confirmReset') - expect(response.statusCode).toBe(200) - expect(response.headers.location).toBe('/account/requestPasswordReset') + const response = await testSession.get('/account/password/confirmReset') + expect(response.statusCode).toBe(200) + expect(response.headers.location).toBe('/account/requestPasswordReset') }) test('redirect old pw-reset routes', async () => { From 22cae7d1fb4d74644cc0bacab091b7adab0b808d Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sat, 2 Dec 2023 21:44:12 -0500 Subject: [PATCH 12/24] fix up testing --- tests/integration/accountRouter.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration/accountRouter.test.js b/tests/integration/accountRouter.test.js index 90730aa7..66adc70d 100644 --- a/tests/integration/accountRouter.test.js +++ b/tests/integration/accountRouter.test.js @@ -24,13 +24,12 @@ describe('Account Routes', function () { test('responds with OK to provided parameters', async () => { const response = await testSession.get('/account/password/confirmReset?username=turbo2&token=XXXXX') expect(response.statusCode).toBe(200) - expect(response.headers.location).toBe('/account/requestPasswordReset') }) - test('redirect to reset request page if missing parameters', async () => { + test('redirect to reset request page if missing parameters with flash parameter', async () => { const response = await testSession.get('/account/password/confirmReset') - expect(response.statusCode).toBe(200) - expect(response.headers.location).toBe('/account/requestPasswordReset') + expect(response.statusCode).toBe(302) + expect(response.headers.location).toContain('/account/requestPasswordReset?flash=') }) test('redirect old pw-reset routes', async () => { From 2de4075c83415fc55d7734c4d6882a6352c8b5c7 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sun, 3 Dec 2023 20:02:18 -0500 Subject: [PATCH 13/24] refactor to use express-validation --- .../views/account/get/confirmPasswordReset.js | 43 +++++++++---------- src/backend/routes/views/accountRouter.js | 8 +++- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index 8eee3034..3dbcd215 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -1,40 +1,37 @@ +const { validationResult } = require('express-validator'); exports = module.exports = function (req, res) { const locals = res.locals const overallRes = res - // locals.section is used to set the currently selected - // item in the header navigation. - locals.section = 'account' - - locals.formData = req.body || {} - - // Render the view - locals.username = req.query.username - locals.token = req.query.token - - // Ensure token and username are present - const validateParamPresence = (token, username) => { - if (!token || !username) { - return 'Invalid request' - } else { - return null - } - } + const errors = validationResult(req) - const errorMsg = validateParamPresence(req.query.username, req.query.token) + console.log(errors) - if (errorMsg) { + if (!errors.isEmpty()) { const flash = {} flash.class = 'alert-danger' - flash.messages = [{ msg: errorMsg }] flash.type = 'Error!' + flash.messages = [] + errors.array().forEach((e) => { + flash.messages.push({ msg: e.msg }) + }) const buff = Buffer.from(JSON.stringify(flash)) const data = buff.toString('base64') return overallRes.redirect('/account/requestPasswordReset?flash=' + data) - } else { - res.render('account/confirmPasswordReset') } + + // locals.section is used to set the currently selected + // item in the header navigation. + locals.section = 'account' + + locals.formData = req.body || {} + + // Render the view + locals.username = req.query.username + locals.token = req.query.token + + return res.render('account/confirmPasswordReset') } diff --git a/src/backend/routes/views/accountRouter.js b/src/backend/routes/views/accountRouter.js index 4a4b8957..62ed1479 100644 --- a/src/backend/routes/views/accountRouter.js +++ b/src/backend/routes/views/accountRouter.js @@ -1,5 +1,7 @@ const express = require('../../ExpressApp') const router = express.Router() + +const { query, validationResult } = require('express-validator'); const middlewares = require('../middleware') const url = require('url') @@ -18,7 +20,11 @@ router.post('/changeEmail', middlewares.isAuthenticated(), require('./account/po router.get('/changeUsername', middlewares.isAuthenticated(), require('./account/get/changeUsername')) router.post('/changeUsername', middlewares.isAuthenticated(), require('./account/post/changeUsername')) -router.get('/password/confirmReset', require('./account/get/confirmPasswordReset')) +router.get('/password/confirmReset', [ + query('token').notEmpty().withMessage('Token is required'), + query('Username').notEmpty().withMessage('Username is required')], + require('./account/get/confirmPasswordReset') + ) router.post('/password/confirmReset', require('./account/post/confirmPasswordReset')) router.get('/requestPasswordReset', require('./account/get/requestPasswordReset')) From e2adad7aa17aa7f54b30a197dda7498099e97870 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sun, 3 Dec 2023 20:04:31 -0500 Subject: [PATCH 14/24] lint --- .../routes/views/account/get/confirmPasswordReset.js | 2 +- src/backend/routes/views/accountRouter.js | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index 3dbcd215..fa9b7fed 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -1,4 +1,4 @@ -const { validationResult } = require('express-validator'); +const { validationResult } = require('express-validator') exports = module.exports = function (req, res) { const locals = res.locals diff --git a/src/backend/routes/views/accountRouter.js b/src/backend/routes/views/accountRouter.js index 62ed1479..354be3cf 100644 --- a/src/backend/routes/views/accountRouter.js +++ b/src/backend/routes/views/accountRouter.js @@ -1,7 +1,7 @@ const express = require('../../ExpressApp') const router = express.Router() -const { query, validationResult } = require('express-validator'); +const { query } = require('express-validator') const middlewares = require('../middleware') const url = require('url') @@ -20,11 +20,9 @@ router.post('/changeEmail', middlewares.isAuthenticated(), require('./account/po router.get('/changeUsername', middlewares.isAuthenticated(), require('./account/get/changeUsername')) router.post('/changeUsername', middlewares.isAuthenticated(), require('./account/post/changeUsername')) -router.get('/password/confirmReset', [ - query('token').notEmpty().withMessage('Token is required'), +router.get('/password/confirmReset', [query('token').notEmpty().withMessage('Token is required'), query('Username').notEmpty().withMessage('Username is required')], - require('./account/get/confirmPasswordReset') - ) +require('./account/get/confirmPasswordReset')) router.post('/password/confirmReset', require('./account/post/confirmPasswordReset')) router.get('/requestPasswordReset', require('./account/get/requestPasswordReset')) From 91b178541d9af344c5cf6a1e73fc5dcf200f81de Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sun, 3 Dec 2023 20:14:20 -0500 Subject: [PATCH 15/24] fix uppercase u --- src/backend/routes/views/accountRouter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/routes/views/accountRouter.js b/src/backend/routes/views/accountRouter.js index 354be3cf..35e83365 100644 --- a/src/backend/routes/views/accountRouter.js +++ b/src/backend/routes/views/accountRouter.js @@ -20,8 +20,8 @@ router.post('/changeEmail', middlewares.isAuthenticated(), require('./account/po router.get('/changeUsername', middlewares.isAuthenticated(), require('./account/get/changeUsername')) router.post('/changeUsername', middlewares.isAuthenticated(), require('./account/post/changeUsername')) -router.get('/password/confirmReset', [query('token').notEmpty().withMessage('Token is required'), - query('Username').notEmpty().withMessage('Username is required')], +router.get('/password/confirmReset', [query('token').notEmpty().withMessage('Invalid reset request'), + query('username').notEmpty().withMessage('Invalid reset request')], require('./account/get/confirmPasswordReset')) router.post('/password/confirmReset', require('./account/post/confirmPasswordReset')) From 4e5872fa3fabcd780c5ea322430e731949fde9ca Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sun, 3 Dec 2023 21:01:33 -0500 Subject: [PATCH 16/24] user express validatior in routes for query params --- .../routes/views/account/get/confirmPasswordReset.js | 6 +----- src/backend/routes/views/accountRouter.js | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index fa9b7fed..0497b5c9 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -2,12 +2,8 @@ const { validationResult } = require('express-validator') exports = module.exports = function (req, res) { const locals = res.locals - const overallRes = res - const errors = validationResult(req) - console.log(errors) - if (!errors.isEmpty()) { const flash = {} flash.class = 'alert-danger' @@ -20,7 +16,7 @@ exports = module.exports = function (req, res) { const buff = Buffer.from(JSON.stringify(flash)) const data = buff.toString('base64') - return overallRes.redirect('/account/requestPasswordReset?flash=' + data) + return res.redirect('/account/requestPasswordReset?flash=' + data) } // locals.section is used to set the currently selected diff --git a/src/backend/routes/views/accountRouter.js b/src/backend/routes/views/accountRouter.js index 35e83365..976ce7f7 100644 --- a/src/backend/routes/views/accountRouter.js +++ b/src/backend/routes/views/accountRouter.js @@ -20,8 +20,8 @@ router.post('/changeEmail', middlewares.isAuthenticated(), require('./account/po router.get('/changeUsername', middlewares.isAuthenticated(), require('./account/get/changeUsername')) router.post('/changeUsername', middlewares.isAuthenticated(), require('./account/post/changeUsername')) -router.get('/password/confirmReset', [query('token').notEmpty().withMessage('Invalid reset request'), - query('username').notEmpty().withMessage('Invalid reset request')], +router.get('/password/confirmReset', [query('token').notEmpty().withMessage('Missing token'), + query('username').notEmpty().withMessage('Missing username')], require('./account/get/confirmPasswordReset')) router.post('/password/confirmReset', require('./account/post/confirmPasswordReset')) From 4065a4b2ced5954f9a8bdcbc47f20627705d413e Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sun, 3 Dec 2023 21:01:50 -0500 Subject: [PATCH 17/24] update tests, add secondary test --- tests/integration/accountRouter.test.js | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/integration/accountRouter.test.js b/tests/integration/accountRouter.test.js index 66adc70d..07870f55 100644 --- a/tests/integration/accountRouter.test.js +++ b/tests/integration/accountRouter.test.js @@ -26,10 +26,28 @@ describe('Account Routes', function () { expect(response.statusCode).toBe(200) }) - test('redirect to reset request page if missing parameters with flash parameter', async () => { - const response = await testSession.get('/account/password/confirmReset') + test('redirect to reset request page if missing username parameter with flash parameter', async () => { + const response = await testSession.get('/account/password/confirmReset?token=XXXXX') + + const flashValue = response.headers.location.match(/flash=([^&]+)/)[1] + const buff = Buffer.from(flashValue, 'base64') + const text = JSON.parse(buff.toString('ascii')).messages[0].msg + + expect(response.statusCode).toBe(302) + expect(response.headers.location).toContain('/account/requestPasswordReset?flash=') + expect(text).toBe('Missing username') + }) + + test('redirect to reset request page if missing token parameter with flash parameter', async () => { + const response = await testSession.get('/account/password/confirmReset?username=turbo2') + + const flashValue = response.headers.location.match(/flash=([^&]+)/)[1] + const buff = Buffer.from(flashValue, 'base64') + const text = JSON.parse(buff.toString('ascii')).messages[0].msg + expect(response.statusCode).toBe(302) expect(response.headers.location).toContain('/account/requestPasswordReset?flash=') + expect(text).toBe('Missing token') }) test('redirect old pw-reset routes', async () => { From 4aba28d744a5a260031ab303ed841970b80b07a7 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Sun, 3 Dec 2023 21:02:28 -0500 Subject: [PATCH 18/24] add logging to docker - prints console.log into docker logs since they dont seem to be forwarded to the browser --- docker-compose.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 28000ad4..44a2dbcc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,7 +12,9 @@ services: context: . networks: - faf-stack - + logging: + driver: "json-file" + networks: faf-stack: name: faf-stack_faf From 02ccdee7bdacabf06d92e09c331db66a26b81831 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Mon, 4 Dec 2023 20:12:06 -0500 Subject: [PATCH 19/24] merge --- .../routes/views/account/get/requestPasswordReset.js | 2 +- tests/integration/accountRouter.test.js | 9 ++++----- tests/setup.js | 4 +++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index e918ae96..43fb3609 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -39,7 +39,7 @@ exports = module.exports = async function (req, res) { section: 'account', flash: { class: 'alert-danger', - messages: 'issue resetting', + messages: [{ msg: 'issue resetting' }], type: 'Error!' }, formData, diff --git a/tests/integration/accountRouter.test.js b/tests/integration/accountRouter.test.js index 07870f55..e500e807 100644 --- a/tests/integration/accountRouter.test.js +++ b/tests/integration/accountRouter.test.js @@ -29,13 +29,12 @@ describe('Account Routes', function () { test('redirect to reset request page if missing username parameter with flash parameter', async () => { const response = await testSession.get('/account/password/confirmReset?token=XXXXX') - const flashValue = response.headers.location.match(/flash=([^&]+)/)[1] - const buff = Buffer.from(flashValue, 'base64') - const text = JSON.parse(buff.toString('ascii')).messages[0].msg - expect(response.statusCode).toBe(302) expect(response.headers.location).toContain('/account/requestPasswordReset?flash=') - expect(text).toBe('Missing username') + + const redirectResponse = await testSession.get(response.headers.location) + + expect(redirectResponse.text).toContain('Missing username') }) test('redirect to reset request page if missing token parameter with flash parameter', async () => { diff --git a/tests/setup.js b/tests/setup.js index 5f0dc168..9076003f 100644 --- a/tests/setup.js +++ b/tests/setup.js @@ -1,7 +1,6 @@ const fs = require('fs') const { WordpressService } = require('../src/backend/services/WordpressService') const { LeaderboardService } = require('../src/backend/services/LeaderboardService') -const { JavaApiM2MClient } = require('../src/backend/services/JavaApiM2MClient') const appConfig = require('../src/backend/config/app') const nock = require('nock') nock.disableNetConnect() @@ -51,6 +50,9 @@ beforeEach(() => { nock(appConfig.apiUrl) .get('/data/clan/2741?include=memberships.player') .reply(200, fs.readFileSync('tests/integration/testData/clan/clan.json', { encoding: 'utf8', flag: 'r' })) + nock(appConfig.apiUrl) + .post('/users/buildSteamPasswordResetUrl') + .reply(200, { steamUrl: 'http://localhost/test-steam-reset' }) }) afterEach(() => { From 27e2ad8237eb32b02ded0e38640bc6f556e256e2 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Mon, 4 Dec 2023 20:12:36 -0500 Subject: [PATCH 20/24] merge --- tests/setup.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/setup.js b/tests/setup.js index 9076003f..308cf6ee 100644 --- a/tests/setup.js +++ b/tests/setup.js @@ -36,6 +36,13 @@ beforeEach(() => { throw new Error('do we need to change the mock?') }) +<<<<<<< HEAD +======= + nock(appConfig.apiUrl) + .post('/users/buildSteamPasswordResetUrl') + .reply(200, { steamUrl: 'http://localhost/test-steam-reset' }) + +>>>>>>> e9a54d1 (lint) jest.spyOn(JavaApiM2MClient, 'getToken').mockResolvedValue({ token: { refresh: () => {}, From d60bf70d8ff6cf6379191ab305d1b3f867cab26a Mon Sep 17 00:00:00 2001 From: beckpaul Date: Mon, 4 Dec 2023 20:12:59 -0500 Subject: [PATCH 21/24] merge --- tests/setup.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/setup.js b/tests/setup.js index 308cf6ee..a2ad4ad8 100644 --- a/tests/setup.js +++ b/tests/setup.js @@ -36,13 +36,10 @@ beforeEach(() => { throw new Error('do we need to change the mock?') }) -<<<<<<< HEAD -======= nock(appConfig.apiUrl) .post('/users/buildSteamPasswordResetUrl') .reply(200, { steamUrl: 'http://localhost/test-steam-reset' }) ->>>>>>> e9a54d1 (lint) jest.spyOn(JavaApiM2MClient, 'getToken').mockResolvedValue({ token: { refresh: () => {}, From 4959061736068f073b572f1c5c6db0e065085a2b Mon Sep 17 00:00:00 2001 From: beckpaul Date: Mon, 4 Dec 2023 21:29:50 -0500 Subject: [PATCH 22/24] use new linting methodology - refactors --- .../views/account/get/confirmPasswordReset.js | 64 ++++++++++++------- .../views/account/get/requestPasswordReset.js | 19 +----- .../views/account/requestPasswordReset.pug | 4 +- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index 0497b5c9..bb40880d 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -1,33 +1,49 @@ +const axios = require('axios') +const appConfig = require('../../../../config/app') const { validationResult } = require('express-validator') exports = module.exports = function (req, res) { - const locals = res.locals - const errors = validationResult(req) if (!errors.isEmpty()) { - const flash = {} - flash.class = 'alert-danger' - flash.type = 'Error!' - flash.messages = [] - errors.array().forEach((e) => { - flash.messages.push({ msg: e.msg }) - }) - - const buff = Buffer.from(JSON.stringify(flash)) - const data = buff.toString('base64') - - return res.redirect('/account/requestPasswordReset?flash=' + data) + return renderRequestPasswordReset(req, res, errors) } - // locals.section is used to set the currently selected - // item in the header navigation. - locals.section = 'account' - - locals.formData = req.body || {} - - // Render the view - locals.username = req.query.username - locals.token = req.query.token + res.render('account/confirmPasswordReset', { + section: 'account', + formData: req.body || {}, + username: req.query.username, + token: req.query.token + }) +} - return res.render('account/confirmPasswordReset') +const renderRequestPasswordReset = async (req, res, errors) => { + axios.post(appConfig.apiUrl + '/users/buildSteamPasswordResetUrl', {}, { maxRedirects: 0 }).then(response => { + if (response.status !== 200) { + throw new Error('java-api error') + } + + return res.render('account/requestPasswordReset', { + section: 'account', + errors: { + class: 'alert-danger', + messages: errors, + type: 'Error!' + }, + steamReset: response.data.steamUrl, + formData: {}, + recaptchaSiteKey: appConfig.recaptchaKey + }) + }).catch(error => { + console.error(error.toString()) + return res.render('account/requestPasswordReset', { + section: 'account', + errors: { + class: 'alert-danger', + messages: error.toString(), + type: 'Error!' + }, + formData: {}, + recaptchaSiteKey: appConfig.recaptchaKey + }) + }) } diff --git a/src/backend/routes/views/account/get/requestPasswordReset.js b/src/backend/routes/views/account/get/requestPasswordReset.js index 43fb3609..4505ed01 100644 --- a/src/backend/routes/views/account/get/requestPasswordReset.js +++ b/src/backend/routes/views/account/get/requestPasswordReset.js @@ -12,23 +12,8 @@ exports = module.exports = async function (req, res) { throw new Error('java-api error') } - let flash = {} - if (req.query.flash) { - const buff = Buffer.from(req.query.flash, 'base64') - const text = buff.toString('ascii') - - try { - flash = JSON.parse(text) - } catch (e) { - console.error('Parsing error while trying to decode a flash error: ' + text) - console.error(e) - flash = [{ msg: 'Unknown error' }] - } - } - res.render('account/requestPasswordReset', { section: 'account', - flash, steamReset: response.data.steamUrl, formData, recaptchaSiteKey: appConfig.recaptchaKey @@ -37,9 +22,9 @@ exports = module.exports = async function (req, res) { console.error(error.toString()) res.render('account/requestPasswordReset', { section: 'account', - flash: { + errors: { class: 'alert-danger', - messages: [{ msg: 'issue resetting' }], + messages: error.toString, type: 'Error!' }, formData, diff --git a/src/backend/templates/views/account/requestPasswordReset.pug b/src/backend/templates/views/account/requestPasswordReset.pug index 03fc9624..8e6f7e16 100644 --- a/src/backend/templates/views/account/requestPasswordReset.pug +++ b/src/backend/templates/views/account/requestPasswordReset.pug @@ -1,5 +1,5 @@ extends ../../layouts/default -include ../../mixins/flash-messages +include ../../mixins/flash-error include ../../mixins/form/account block bannerMixin block bannerData @@ -9,7 +9,7 @@ block bannerData block content .passResetContainer .flashMessage.column12 - +flash-messages(flash) + +flash-error(errors) .passResetEmail.column12 h1 Reset password via email p Enter your username or email below to reset your password. From 2514fa63e30de9f2bd50893b255e73fe3b478c87 Mon Sep 17 00:00:00 2001 From: beckpaul Date: Mon, 4 Dec 2023 21:45:09 -0500 Subject: [PATCH 23/24] comments and append to error for clarity --- src/backend/routes/views/account/get/confirmPasswordReset.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/backend/routes/views/account/get/confirmPasswordReset.js b/src/backend/routes/views/account/get/confirmPasswordReset.js index bb40880d..5b7d9017 100644 --- a/src/backend/routes/views/account/get/confirmPasswordReset.js +++ b/src/backend/routes/views/account/get/confirmPasswordReset.js @@ -4,6 +4,8 @@ const { validationResult } = require('express-validator') exports = module.exports = function (req, res) { const errors = validationResult(req) + // A render/redirect ignores this if not async and renders the confirm + // Theres probably a better way to do this if (!errors.isEmpty()) { return renderRequestPasswordReset(req, res, errors) } @@ -22,6 +24,8 @@ const renderRequestPasswordReset = async (req, res, errors) => { throw new Error('java-api error') } + errors.errors[errors.errors.length - 1].msg += '. You may request a new link here' + return res.render('account/requestPasswordReset', { section: 'account', errors: { From fc0e862017671672609b7ea864c5414593ae600c Mon Sep 17 00:00:00 2001 From: beckpaul Date: Mon, 4 Dec 2023 21:53:34 -0500 Subject: [PATCH 24/24] organize setup a bit - fix tests --- tests/integration/accountRouter.test.js | 23 +++++++---------------- tests/setup.js | 12 +++++------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/tests/integration/accountRouter.test.js b/tests/integration/accountRouter.test.js index e500e807..3a266d22 100644 --- a/tests/integration/accountRouter.test.js +++ b/tests/integration/accountRouter.test.js @@ -26,27 +26,18 @@ describe('Account Routes', function () { expect(response.statusCode).toBe(200) }) - test('redirect to reset request page if missing username parameter with flash parameter', async () => { + test('render request content if missing username parameter with flash', async () => { const response = await testSession.get('/account/password/confirmReset?token=XXXXX') - expect(response.statusCode).toBe(302) - expect(response.headers.location).toContain('/account/requestPasswordReset?flash=') - - const redirectResponse = await testSession.get(response.headers.location) - - expect(redirectResponse.text).toContain('Missing username') + expect(response.statusCode).toBe(200) + expect(response.text).toContain('Missing username') }) - test('redirect to reset request page if missing token parameter with flash parameter', async () => { - const response = await testSession.get('/account/password/confirmReset?username=turbo2') - - const flashValue = response.headers.location.match(/flash=([^&]+)/)[1] - const buff = Buffer.from(flashValue, 'base64') - const text = JSON.parse(buff.toString('ascii')).messages[0].msg + test('render request content if missing token parameter with flash', async () => { + const response = await testSession.get('/account/password/confirmReset?token=XXXXX') - expect(response.statusCode).toBe(302) - expect(response.headers.location).toContain('/account/requestPasswordReset?flash=') - expect(text).toBe('Missing token') + expect(response.statusCode).toBe(200) + expect(response.text).toContain('Missing username') }) test('redirect old pw-reset routes', async () => { diff --git a/tests/setup.js b/tests/setup.js index a2ad4ad8..87089a37 100644 --- a/tests/setup.js +++ b/tests/setup.js @@ -1,6 +1,7 @@ const fs = require('fs') const { WordpressService } = require('../src/backend/services/WordpressService') const { LeaderboardService } = require('../src/backend/services/LeaderboardService') +const { JavaApiM2MClient } = require('../src/backend/services/JavaApiM2MClient') const appConfig = require('../src/backend/config/app') const nock = require('nock') nock.disableNetConnect() @@ -36,10 +37,6 @@ beforeEach(() => { throw new Error('do we need to change the mock?') }) - nock(appConfig.apiUrl) - .post('/users/buildSteamPasswordResetUrl') - .reply(200, { steamUrl: 'http://localhost/test-steam-reset' }) - jest.spyOn(JavaApiM2MClient, 'getToken').mockResolvedValue({ token: { refresh: () => {}, @@ -47,6 +44,10 @@ beforeEach(() => { } }) + nock(appConfig.apiUrl) + .post('/users/buildSteamPasswordResetUrl') + .reply(200, { steamUrl: 'http://localhost/test-steam-reset' }) + nock(appConfig.apiUrl) .get('/data/clan?include=leader&fields[clan]=name,tag,description,leader,memberships,createTime&fields[player]=login&page[number]=1&page[size]=3000') .reply(200, fs.readFileSync('tests/integration/testData/clan/clans.json', { encoding: 'utf8', flag: 'r' })) @@ -54,9 +55,6 @@ beforeEach(() => { nock(appConfig.apiUrl) .get('/data/clan/2741?include=memberships.player') .reply(200, fs.readFileSync('tests/integration/testData/clan/clan.json', { encoding: 'utf8', flag: 'r' })) - nock(appConfig.apiUrl) - .post('/users/buildSteamPasswordResetUrl') - .reply(200, { steamUrl: 'http://localhost/test-steam-reset' }) }) afterEach(() => {