Parameter validation for /account/password/confirmReset #515

.gitignore

@@ -52,3 +52,4 @@ public/js/*.js
 sessions
 
 dist
+.tool-versions

docker-compose.yml

@@ -12,7 +12,9 @@ services:
       context: .
     networks:
       -  faf-stack
-
+    logging:
+      driver: "json-file"
+
 networks:
   faf-stack:
     name: faf-stack_faf

src/backend/routes/views/account/get/confirmPasswordReset.js

@@ -1,16 +1,53 @@
+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)
 
-    // locals.section is used to set the currently selected
-    // item in the header navigation.
-    locals.section = 'account'
+    // 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)
+    }
 
-    locals.formData = req.body || {}
+    res.render('account/confirmPasswordReset', {
+        section: 'account',
+        formData: req.body || {},
+        username: req.query.username,
+        token: req.query.token
+    })
+}
+
+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')
+        }
 
-    const flash = null
+        errors.errors[errors.errors.length - 1].msg += '. You may request a new link here'
 
-    // Render the view
-    locals.username = req.query.username
-    locals.token = req.query.token
-    res.render('account/confirmPasswordReset', { flash })
+        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
+        })
+    })
 }

src/backend/routes/views/account/get/requestPasswordReset.js

@@ -14,7 +14,6 @@ exports = module.exports = async function (req, res) {
 
         res.render('account/requestPasswordReset', {
             section: 'account',
-            flash: {},
             steamReset: response.data.steamUrl,
             formData,
             recaptchaSiteKey: appConfig.recaptchaKey
@@ -23,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: 'issue resetting',
+                messages: error.toString,
                 type: 'Error!'
             },
             formData,

src/backend/routes/views/accountRouter.js

@@ -1,5 +1,7 @@
 const express = require('../../ExpressApp')
 const router = express.Router()
+
+const { query } = require('express-validator')
 const middlewares = require('../middleware')
 const url = require('url')
 
@@ -18,7 +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', require('./account/get/confirmPasswordReset'))
+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'))
 
 router.get('/requestPasswordReset', require('./account/get/requestPasswordReset'))

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

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.
@@ -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
 

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,25 @@ 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)
+    })
+
+    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(200)
+        expect(response.text).toContain('Missing username')
+    })
+
+    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(200)
+        expect(response.text).toContain('Missing username')
+    })
+
     test('redirect old pw-reset routes', async () => {
         const response = await testSession.get('/account/password/reset')
         expect(response.statusCode).toBe(302)

tests/setup.js

@@ -44,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' }))