diff --git a/packages/server/server/controllers/auth.js b/packages/server/server/controllers/auth.js index 6017abd..eeee50c 100644 --- a/packages/server/server/controllers/auth.js +++ b/packages/server/server/controllers/auth.js @@ -2,12 +2,21 @@ const knex = require('../data/db') const { hashPassword, compareHash } = require('../services/bcrypt'); const signToken = require('../services/signToken'); -const { validateSignup, validateLogin } = require('../services/userValidate'); + +const { validationResult } = require('express-validator'); + +const checkValidationErrors = (req, res) => { + const errors = validationResult(req); + if (!errors.isEmpty()) { + return res.status(422).json({ errors: errors.array() }); + } +} const signup = async (req, res, next) => { + checkValidationErrors(req, res); + const user = req.body; - if (!validateSignup(user)) return; try { @@ -15,6 +24,7 @@ const signup = async (req, res, next) => { const secureUser = { ...user, password: hashedPassword } knex('user') + .returning(['username', 'email']) .insert(secureUser) .then(queryResults => { const newUser = queryResults[0]; @@ -28,9 +38,10 @@ const signup = async (req, res, next) => { } const login = async (req, res, next) => { - + + checkValidationErrors(req, res); + const user = req.body; - if (!validateLogin(user)) return; try { diff --git a/packages/server/server/middleware/userValidator.js b/packages/server/server/middleware/userValidator.js new file mode 100644 index 0000000..d816404 --- /dev/null +++ b/packages/server/server/middleware/userValidator.js @@ -0,0 +1,36 @@ +const {check, sanitize, validationResult} = require('express-validator'); + +const signupValidationRules = () => { + return [ + check('email', 'invalid email').normalizeEmail().isEmail(), + check('username', 'invalid username').isString(), + check('password', 'invalid password').isString().isLength({min: 8}), + sanitize('username').escape() + ] +} + +const loginValidationRules = () => { + return [ + check('username', 'bad credentials').isString(), + check('password', 'bad credentials').isString() + ] +} + +const validate = (req, res, next) => { + const errors = validationResult(req) + if (errors.isEmpty()) { + return next() + } + const extractedErrors = [] + errors.array().map(err => extractedErrors.push({ [err.param]: err.msg })) + + return res.status(422).json({ + errors: extractedErrors, + }) +} + +module.exports = { + signupValidationRules, + loginValidationRules, + validate +} \ No newline at end of file diff --git a/packages/server/server/package-lock.json b/packages/server/server/package-lock.json index 3d4a99f..7714583 100644 --- a/packages/server/server/package-lock.json +++ b/packages/server/server/package-lock.json @@ -1043,6 +1043,15 @@ "vary": "~1.1.2" } }, + "express-validator": { + "version": "6.3.1", + "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-6.3.1.tgz", + "integrity": "sha512-YQHQKP/zlUTN6d38uWwXgK3At5phK6R24pOB/ImWisMUz/U/1AC3ZXMgiZYhtH4ViYJ6UAiV0/nj8s1Qs3kmvw==", + "requires": { + "lodash": "^4.17.15", + "validator": "^11.1.0" + } + }, "extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", @@ -3461,6 +3470,11 @@ "homedir-polyfill": "^1.0.1" } }, + "validator": { + "version": "11.1.0", + "resolved": "https://registry.npmjs.org/validator/-/validator-11.1.0.tgz", + "integrity": "sha512-qiQ5ktdO7CD6C/5/mYV4jku/7qnqzjrxb3C/Q5wR3vGGinHTgJZN/TdFT3ZX4vXhX2R1PXx42fB1cn5W+uJ4lg==" + }, "vary": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/vary/-/vary-1.1.2.tgz", diff --git a/packages/server/server/package.json b/packages/server/server/package.json index f332bd1..1281016 100644 --- a/packages/server/server/package.json +++ b/packages/server/server/package.json @@ -17,6 +17,7 @@ "debug": "~2.6.9", "dotenv": "^8.2.0", "express": "~4.16.1", + "express-validator": "^6.3.1", "http-errors": "~1.6.3", "jsonwebtoken": "^8.5.1", "knex": "^0.20.7", diff --git a/packages/server/server/routes/auth.js b/packages/server/server/routes/auth.js index b521ecc..dfa96f9 100644 --- a/packages/server/server/routes/auth.js +++ b/packages/server/server/routes/auth.js @@ -1,9 +1,10 @@ const express = require('express'); const router = express.Router(); - +const app = require('../server'); const authController = require('../controllers/auth'); +const { signupValidationRules, loginValidationRules, validate } = require('../middleware/userValidator'); -router.post('/signup', authController.signup); -router.post('/login', authController.login); +router.post('/signup', signupValidationRules(), validate, authController.signup); +router.post('/login', loginValidationRules(), validate, authController.login); module.exports = router; diff --git a/packages/server/server/services/userValidate.js b/packages/server/server/services/userValidate.js deleted file mode 100644 index 19dda00..0000000 --- a/packages/server/server/services/userValidate.js +++ /dev/null @@ -1,14 +0,0 @@ -const validateSignup = (user) => { - if (!user.username) throw('no username'); - if (!user.email) throw('no email'); - if (!user.password) throw('no password'); - return true -} - -const validateLogin = (user) => { - if (!user.username) throw('no username'); - if (!user.password) throw('no password'); - return true; -} - -module.exports = { validateLogin, validateSignup }; \ No newline at end of file diff --git a/packages/server/server/services/verifyToken.js b/packages/server/server/services/verifyToken.js new file mode 100644 index 0000000..13444e3 --- /dev/null +++ b/packages/server/server/services/verifyToken.js @@ -0,0 +1,13 @@ +const jwt = require('jsonwebtoken'); +require('dotenv').config(); + +const verifyToken = (token) => { + const secret = process.env.NODE_ENV === 'test' ? process.env.TEST_SECRET : process.env.JWT_SECRET; + const decoded = jwt.verify(token, secret, (err, decoded) => { + if (err) return err; + return decoded; + }); + return decoded; +} + +module.exports = verifyToken; \ No newline at end of file diff --git a/packages/server/server/test/auth.login.spec.js b/packages/server/server/test/auth.login.spec.js new file mode 100644 index 0000000..78ebcf8 --- /dev/null +++ b/packages/server/server/test/auth.login.spec.js @@ -0,0 +1,63 @@ +const authSignupSpec = (chai, knex, server) => { + const newUserFormData = { + 'username':'newUser', + 'password':'password', + 'email':'user@example.com' + } + const loginFormData = { + 'username':'newUser', + 'password':'password' + } + + + it('post to /login with non-registered user should return status 401 with bad creds err', done => { + chai.request(server) + .post('/auth/login') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + res.should.status(401); + res.body.err.should.equal('bad credentials'); + done(); + }); + }) + + it('post to /login with non-registered user should return status 401 with bad creds err', done => { + chai.request(server) + .post('/auth/login') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + res.should.status(401); + res.body.err.should.equal('bad credentials'); + done(); + }) + }) + + it('post to /login with registered user should return cookie', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + + chai.request(server) + .post('/auth/login') + .type('form') + .send(loginFormData) + .end((err, res) => { + if(err) done(err); + res.should.status(200); + res.should.cookie('token'); + done(); + }) + }) + }) + + + +} +module.exports = authSignupSpec; \ No newline at end of file diff --git a/packages/server/server/test/auth.signup.spec.js b/packages/server/server/test/auth.signup.spec.js new file mode 100644 index 0000000..218a98f --- /dev/null +++ b/packages/server/server/test/auth.signup.spec.js @@ -0,0 +1,144 @@ +const verifyToken = require('../services/verifyToken'); + +const authSignupSpec = (chai, knex, server) => { + const newUserFormData = { + 'username':'newUser', + 'password':'password', + 'email':'user@example.com' + } + const invalidEmailFormData = { + 'username':'newUser', + 'email': 'useremail', + 'password':'password' + } + const scriptInjectionFormData = { + 'username': '', + 'password':'password', + 'email':'user@example.com' + } + const sqlInjectionFormData = { + 'username': '; DROP TABLE user;', + 'password':'password', + 'email':'user@example.com' + } + + it('post to /signup should return 200 status', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + res.should.status(200); + done(); + }); + }); + + it('post to /signup should return token', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + res.should.cookie('token'); + done(); + }); + }); + + it('post to /signup should add user to db', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + knex('user').where({'username': newUserFormData.username}).then(results => { + const newUser = results[0]; + if (newUser.username === newUserFormData.username) done(); + }) + }); + }) + + it('post to /signup should add user to db with password', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + knex('user').where({'username': newUserFormData.username}).then(results => { + const newUser = results[0]; + if (newUser.password !== newUserFormData.password) done(); + }) + }); + }); + + it('post to /signup with invalid email should return 422', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(invalidEmailFormData) + .end((err, res) => { + if (err) done(err); + res.should.status(422); + done(); + }); + }) + + it('post to /signup should return cookie with jwt for user', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(newUserFormData) + .end((err, res) => { + if (err) done(err); + const cookie = res.headers['set-cookie'][0]; + const token = cookie.split(';')[0].split('token=')[1] + const verifiedToken = verifyToken(token); + const userAssertion = verifiedToken.should.have.property('user'); + userAssertion.with.property('username'); + userAssertion.with.property('email'); + userAssertion.not.with.property('password'); + done() + }) + }) + + it('post to /signup should sanitize inputs for script injection', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(scriptInjectionFormData) + .end((err, res) => { + if (err) done(err); + const cookie = res.headers['set-cookie'][0]; + const token = cookie.split(';')[0].split('token=')[1] + const verifiedToken = verifyToken(token); + verifiedToken.should.have.property('user') + .with.property('username') + .to.not.equal('') + done() + }) + }) + + it('post to /signup should sanitize inputs for sql injection', done => { + chai.request(server) + .post('/auth/signup') + .type('form') + .send(sqlInjectionFormData) + .end((err, res) => { + if (err) done(err); + knex('user') + .where('id', 1) + .select('id','username','email') + .then(results => { + const newUser = results[0]; + if (newUser) done(); + }) + }) + }) + + + +} +module.exports = authSignupSpec; \ No newline at end of file diff --git a/packages/server/server/test/auth.spec.js b/packages/server/server/test/auth.spec.js deleted file mode 100644 index 383d04b..0000000 --- a/packages/server/server/test/auth.spec.js +++ /dev/null @@ -1,114 +0,0 @@ -const authSpec = (chai, knex, server) => { - const newUserFormData = { - 'username':'newUser', - 'password':'password', - 'email':'user@example.com' - } - const loginFormData = { - 'username':'newUser', - 'password':'password' - } - - it('post to sign up should return 200 status', done => { - chai.request(server) - .post('/auth/signup') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - res.should.status(200); - done(); - }); - }); - - it('post to sign up should return token', done => { - chai.request(server) - .post('/auth/signup') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - res.should.cookie('token'); - done(); - }); - }); - - it('post to sign up should add user to db', done => { - chai.request(server) - .post('/auth/signup') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - knex('user').where({'username': newUserFormData.username}).then(results => { - const newUser = results[0]; - if (newUser.username === newUserFormData.username) done(); - }) - }); - }) - - it('post to sign up should add user to db with password', done => { - chai.request(server) - .post('/auth/signup') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - knex('user').where({'username': newUserFormData.username}).then(results => { - const newUser = results[0]; - if (newUser.password !== newUserFormData.password) done(); - }) - }); - }); - - it('post to login with non-registered user should return status 401 with bad creds err', done => { - chai.request(server) - .post('/auth/login') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - res.should.status(401); - res.body.err.should.equal('bad credentials'); - done(); - }); - }) - - it('post to login with non-registered user should return status 401 with bad creds err', done => { - chai.request(server) - .post('/auth/login') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - res.should.status(401); - res.body.err.should.equal('bad credentials'); - done(); - }) - }) - - it('post to login with registered user should return cookie', done => { - chai.request(server) - .post('/auth/signup') - .type('form') - .send(newUserFormData) - .end((err, res) => { - if (err) done(err); - - chai.request(server) - .post('/auth/login') - .type('form') - .send(loginFormData) - .end((err, res) => { - if(err) done(err); - res.should.status(200); - res.should.cookie('token'); - done(); - }) - }) - }) - - - -} -module.exports = authSpec; \ No newline at end of file diff --git a/packages/server/server/test/spec.js b/packages/server/server/test/spec.js index a304447..2f0ec61 100644 --- a/packages/server/server/test/spec.js +++ b/packages/server/server/test/spec.js @@ -8,7 +8,8 @@ const server = require('../server'); const should = chai.should(); -const authSpec = require('./auth.spec'); +const authSignupSpec = require('./auth.signup.spec'); +const authLoginSpec = require('./auth.login.spec'); chai.use(chaiHttp); // ! to run tests from other testing modules @@ -29,8 +30,8 @@ const setupDb = () => { describe('Auth Routes', function() { setupDb(); - authSpec(chai, knex, server) - + authSignupSpec(chai, knex, server); + authLoginSpec(chai, knex, server); }); describe('API Routes', function() {