Skip to content

Commit db94e34

Browse files
authored
Develop 515 (#516)
* add redirect if either param is missing - working on flash * refactor to be more funcitonal - flashes are harder than ancicipated * add .tool-versions to git ignore - this is used by asdf * less verbose - turnary was being problematic * flashes work with this - however it does not confirm to other standards * found an example of this - working flashes following existing code conventions * safe flash * fix flash persistence * linting * update unit tests * lint again - sorry for wasting minutes my bad * fix up testing * refactor to use express-validation * lint * fix uppercase u * user express validatior in routes for query params * update tests, add secondary test * add logging to docker - prints console.log into docker logs since they dont seem to be forwarded to the browser * merge * merge * merge * use new linting methodology - refactors * comments and append to error for clarity * organize setup a bit - fix tests
1 parent 4e1b874 commit db94e34

File tree

9 files changed

+86
-21
lines changed

9 files changed

+86
-21
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,4 @@ public/js/*.js
5252
sessions
5353

5454
dist
55+
.tool-versions

docker-compose.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ services:
1212
context: .
1313
networks:
1414
- faf-stack
15-
15+
logging:
16+
driver: "json-file"
17+
1618
networks:
1719
faf-stack:
1820
name: faf-stack_faf
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,53 @@
1+
const axios = require('axios')
2+
const appConfig = require('../../../../config/app')
3+
const { validationResult } = require('express-validator')
14
exports = module.exports = function (req, res) {
2-
const locals = res.locals
5+
const errors = validationResult(req)
36

4-
// locals.section is used to set the currently selected
5-
// item in the header navigation.
6-
locals.section = 'account'
7+
// A render/redirect ignores this if not async and renders the confirm
8+
// Theres probably a better way to do this
9+
if (!errors.isEmpty()) {
10+
return renderRequestPasswordReset(req, res, errors)
11+
}
712

8-
locals.formData = req.body || {}
13+
res.render('account/confirmPasswordReset', {
14+
section: 'account',
15+
formData: req.body || {},
16+
username: req.query.username,
17+
token: req.query.token
18+
})
19+
}
20+
21+
const renderRequestPasswordReset = async (req, res, errors) => {
22+
axios.post(appConfig.apiUrl + '/users/buildSteamPasswordResetUrl', {}, { maxRedirects: 0 }).then(response => {
23+
if (response.status !== 200) {
24+
throw new Error('java-api error')
25+
}
926

10-
const flash = null
27+
errors.errors[errors.errors.length - 1].msg += '. You may request a new link here'
1128

12-
// Render the view
13-
locals.username = req.query.username
14-
locals.token = req.query.token
15-
res.render('account/confirmPasswordReset', { flash })
29+
return res.render('account/requestPasswordReset', {
30+
section: 'account',
31+
errors: {
32+
class: 'alert-danger',
33+
messages: errors,
34+
type: 'Error!'
35+
},
36+
steamReset: response.data.steamUrl,
37+
formData: {},
38+
recaptchaSiteKey: appConfig.recaptchaKey
39+
})
40+
}).catch(error => {
41+
console.error(error.toString())
42+
return res.render('account/requestPasswordReset', {
43+
section: 'account',
44+
errors: {
45+
class: 'alert-danger',
46+
messages: error.toString(),
47+
type: 'Error!'
48+
},
49+
formData: {},
50+
recaptchaSiteKey: appConfig.recaptchaKey
51+
})
52+
})
1653
}

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ exports = module.exports = async function (req, res) {
1414

1515
res.render('account/requestPasswordReset', {
1616
section: 'account',
17-
flash: {},
1817
steamReset: response.data.steamUrl,
1918
formData,
2019
recaptchaSiteKey: appConfig.recaptchaKey
@@ -23,9 +22,9 @@ exports = module.exports = async function (req, res) {
2322
console.error(error.toString())
2423
res.render('account/requestPasswordReset', {
2524
section: 'account',
26-
flash: {
25+
errors: {
2726
class: 'alert-danger',
28-
messages: 'issue resetting',
27+
messages: error.toString,
2928
type: 'Error!'
3029
},
3130
formData,

src/backend/routes/views/accountRouter.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
const express = require('../../ExpressApp')
22
const router = express.Router()
3+
4+
const { query } = require('express-validator')
35
const middlewares = require('../middleware')
46
const url = require('url')
57

@@ -18,7 +20,9 @@ router.post('/changeEmail', middlewares.isAuthenticated(), require('./account/po
1820
router.get('/changeUsername', middlewares.isAuthenticated(), require('./account/get/changeUsername'))
1921
router.post('/changeUsername', middlewares.isAuthenticated(), require('./account/post/changeUsername'))
2022

21-
router.get('/password/confirmReset', require('./account/get/confirmPasswordReset'))
23+
router.get('/password/confirmReset', [query('token').notEmpty().withMessage('Missing token'),
24+
query('username').notEmpty().withMessage('Missing username')],
25+
require('./account/get/confirmPasswordReset'))
2226
router.post('/password/confirmReset', require('./account/post/confirmPasswordReset'))
2327

2428
router.get('/requestPasswordReset', require('./account/get/requestPasswordReset'))

src/backend/templates/views/account/confirmPasswordReset.pug

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ block content
1414

1515
.row
1616
.col-md-offset-3.col-md-6
17-
form(method='post',action="/account/password/confirmReset?username="+username+"&token="+token,data-toggle="validator")
17+
form(method='post', action="/account/password/confirmReset?username="+username+"&token="+token, data-toggle="validator")
1818
+confirm-password
1919
.form-actions
2020
button(type='submit').btn.btn-default.btn-lg.btn-outro.btn-danger Reset

src/backend/templates/views/account/requestPasswordReset.pug

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
extends ../../layouts/default
2-
include ../../mixins/flash-messages
2+
include ../../mixins/flash-error
33
include ../../mixins/form/account
44
block bannerMixin
55
block bannerData
@@ -9,7 +9,7 @@ block bannerData
99
block content
1010
.passResetContainer
1111
.flashMessage.column12
12-
+flash-messages(flash)
12+
+flash-error(errors)
1313
.passResetEmail.column12
1414
h1 Reset password via email
1515
p Enter your username or email below to reset your password.
@@ -27,11 +27,11 @@ block content
2727
label.column12
2828
.g-recaptcha(data-sitekey=recaptchaSiteKey)
2929
.form-actions
30-
30+
3131
button(type='submit').btn.btn-default.btn-lg.btn-outro.btn-danger Reset via email
3232
br
3333
br
34-
34+
3535
br
3636
br
3737

tests/integration/accountRouter.test.js

+19-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ beforeEach(async () => {
1212
describe('Account Routes', function () {
1313
const publicUrls = [
1414
'/account/requestPasswordReset',
15-
'/account/password/confirmReset',
1615
'/account/register',
1716
'/account/activate'
1817
]
@@ -22,6 +21,25 @@ describe('Account Routes', function () {
2221
expect(res.statusCode).toBe(200)
2322
})
2423

24+
test('responds with OK to provided parameters', async () => {
25+
const response = await testSession.get('/account/password/confirmReset?username=turbo2&token=XXXXX')
26+
expect(response.statusCode).toBe(200)
27+
})
28+
29+
test('render request content if missing username parameter with flash', async () => {
30+
const response = await testSession.get('/account/password/confirmReset?token=XXXXX')
31+
32+
expect(response.statusCode).toBe(200)
33+
expect(response.text).toContain('Missing username')
34+
})
35+
36+
test('render request content if missing token parameter with flash', async () => {
37+
const response = await testSession.get('/account/password/confirmReset?token=XXXXX')
38+
39+
expect(response.statusCode).toBe(200)
40+
expect(response.text).toContain('Missing username')
41+
})
42+
2543
test('redirect old pw-reset routes', async () => {
2644
const response = await testSession.get('/account/password/reset')
2745
expect(response.statusCode).toBe(302)

tests/setup.js

+4
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ beforeEach(() => {
4444
}
4545
})
4646

47+
nock(appConfig.apiUrl)
48+
.post('/users/buildSteamPasswordResetUrl')
49+
.reply(200, { steamUrl: 'http://localhost/test-steam-reset' })
50+
4751
nock(appConfig.apiUrl)
4852
.get('/data/clan?include=leader&fields[clan]=name,tag,description,leader,memberships,createTime&fields[player]=login&page[number]=1&page[size]=3000')
4953
.reply(200, fs.readFileSync('tests/integration/testData/clan/clans.json', { encoding: 'utf8', flag: 'r' }))

0 commit comments

Comments
 (0)