From 8e4dcab2f91946bca1ebc22fb254958df0951285 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Thu, 21 Jul 2016 15:07:05 -0700 Subject: [PATCH 01/15] Added authorization check for routes in lib/auth.js and inserted authorization check into projects route --- lib/auth.js | 28 +++++++++++++++++++++++++++- routes/projects.js | 3 ++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index dbd22ed..ae67d40 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -20,7 +20,33 @@ function validate (req) { teamName !== ''); } +/** + * Checks the user credentials. Redirects if the credentials are not valid. + */ +function isAuthenticated (req, res, next) { + 'use strict'; + + let username = req.session.username; + let password = req.session.password; + let teamName = req.session.teamName; + + if (username && password && teamName) { + if (username !== '' && username.length <= 200 && + password.length >= 6 && password.length <= 200 && + teamName !== '') { + next(); // Continue logic flow as normal + } + } + + let authError = new Error('Authentication Failed', 401); + + console.log(authError); + req.session.error = authError; // Passes the error message to the redirected page + res.redirect('/'); +} + module.exports = { authenticate: authenticate, - validate: validate + validate: validate, + isAuthenticated: isAuthenticated }; diff --git a/routes/projects.js b/routes/projects.js index bdc8315..0cedb11 100644 --- a/routes/projects.js +++ b/routes/projects.js @@ -1,8 +1,9 @@ let express = require('express'); let router = express.Router(); +let auth = require('../lib/auth'); /* GET home page. */ -router.get('/', function (req, res, next) { +router.get('/', auth.isAuthenticated, function (req, res, next) { res.render('projects', {title: 'Projects', projects: req.session.projects ? req.session.projects : null}); }); From 70c093ac3774bd412e684a55e95023c7f3936545 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Thu, 21 Jul 2016 15:10:02 -0700 Subject: [PATCH 02/15] Added a note about the session error variable to isAuthenticated function --- lib/auth.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/auth.js b/lib/auth.js index ae67d40..9e29c60 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -22,6 +22,8 @@ function validate (req) { /** * Checks the user credentials. Redirects if the credentials are not valid. + * Note - req.session.error needs to be deleted after the message is displayed + * on redirected page */ function isAuthenticated (req, res, next) { 'use strict'; From f60cbeac0bb93d769b58c86e616e5825ea580381 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Thu, 21 Jul 2016 16:04:01 -0700 Subject: [PATCH 03/15] Tests for isAuthenticated function partially complete --- test/lib/auth.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/lib/auth.js b/test/lib/auth.js index ebc4b79..ef931ba 100644 --- a/test/lib/auth.js +++ b/test/lib/auth.js @@ -162,4 +162,21 @@ describe('Auth module', function () { expect(auth.authenticate(username, password, teamName)).to.be.rejected(); }); }); + + describe('isAuthenticated function', () => { + let credentialsFixture = { body: {} }; + + it('should continue to specified route when user is authenticated', () => { + expect(res.session.username).to.equal('dummy'); + expect(res.session.password).to.equal('password'); + expect(res.session.teamName).to.equal('sevensource'); + }); + + it('should redirect to index page with session variable error code 401' + + 'using invalid credentials', function () { + // Place code here + expect(res).to.redirectTo('/'); + expect(res.session.error.id).to.equal(401); + }); + }) }); From f2e62544ca3dc49e865b2fc21fee16facb38be08 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Fri, 22 Jul 2016 10:39:55 -0700 Subject: [PATCH 04/15] Changed the error message handling. --- lib/auth.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index 9e29c60..0048648 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -40,10 +40,11 @@ function isAuthenticated (req, res, next) { } } - let authError = new Error('Authentication Failed', 401); + let authError = new Error('Authentication failed on projects page', 401); console.log(authError); - req.session.error = authError; // Passes the error message to the redirected page + req.session.error = authError.message; // Passes the error message to the redirected page + console.log('passed authError'); res.redirect('/'); } From 7e758e652beee183e24a62911fa38d15f646bf95 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Fri, 22 Jul 2016 10:40:31 -0700 Subject: [PATCH 05/15] Removed extraneous console.log(). --- lib/auth.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/auth.js b/lib/auth.js index 0048648..765b6f6 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -44,7 +44,6 @@ function isAuthenticated (req, res, next) { console.log(authError); req.session.error = authError.message; // Passes the error message to the redirected page - console.log('passed authError'); res.redirect('/'); } From a20a1212b218257be5f18ee9e446ba34e2bfba40 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Fri, 22 Jul 2016 13:54:13 -0700 Subject: [PATCH 06/15] Updated the middleware to be at the app level and changed the isAuthenticated method to set isAuthenticated value to true or false. Also added session requirement to auth --- app.js | 3 +++ lib/auth.js | 18 ++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app.js b/app.js index bd97a72..cd4860f 100644 --- a/app.js +++ b/app.js @@ -8,6 +8,7 @@ let bodyParser = require('body-parser'); // TODO: Refactor these into a single routes module let routes = require('./routes/index'); let projects = require('./routes/projects'); +let auth = require('./lib/auth'); let app = express(); @@ -30,6 +31,8 @@ app.use(express.static(path.join(__dirname, 'public'))); app.use('/', routes); app.use('/projects', projects); +app.use(auth.isAuthenticated); // To flag the user as authenticated or not + // catch 404 and forward to error handler app.use(function (req, res, next) { let err = new Error('Not Found'); diff --git a/lib/auth.js b/lib/auth.js index 765b6f6..3c35419 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -1,3 +1,5 @@ +let session = require('express-session'); + 'use strict'; let pagination = require('./pagination'); @@ -21,9 +23,7 @@ function validate (req) { } /** - * Checks the user credentials. Redirects if the credentials are not valid. - * Note - req.session.error needs to be deleted after the message is displayed - * on redirected page + * Checks the user credentials. Sets the isAuthenticated flag. */ function isAuthenticated (req, res, next) { 'use strict'; @@ -31,20 +31,18 @@ function isAuthenticated (req, res, next) { let username = req.session.username; let password = req.session.password; let teamName = req.session.teamName; + let isAuth = false; - if (username && password && teamName) { + if (username !== undefined && password !== undefined && teamName !== undefined) { if (username !== '' && username.length <= 200 && password.length >= 6 && password.length <= 200 && teamName !== '') { - next(); // Continue logic flow as normal + isAuth = true; } } - let authError = new Error('Authentication failed on projects page', 401); - - console.log(authError); - req.session.error = authError.message; // Passes the error message to the redirected page - res.redirect('/'); + req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag + next(); // Continue logic flow as normal } module.exports = { From bda50c9d1fa1d1d465a914361e11cf44c140d814 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Fri, 22 Jul 2016 14:00:51 -0700 Subject: [PATCH 07/15] Removed the session reqauired from auth due to lint errors --- lib/auth.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index 3c35419..08931a8 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -1,5 +1,3 @@ -let session = require('express-session'); - 'use strict'; let pagination = require('./pagination'); From 0ae8a4b1425bde4c424b1ea20f81509c722902f2 Mon Sep 17 00:00:00 2001 From: Ruben Date: Mon, 25 Jul 2016 10:51:40 -0700 Subject: [PATCH 08/15] Updated the isAuthenticated logic to check authentication with JamaCloud services --- lib/auth.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index 08931a8..d86f7e4 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -1,5 +1,6 @@ 'use strict'; let pagination = require('./pagination'); +let request = require('request'); // TODO: Separate responsibilities of authenticate into two parts: (1) log-in and (2) return projects function authenticate (username, password, teamName) { @@ -21,7 +22,9 @@ function validate (req) { } /** - * Checks the user credentials. Sets the isAuthenticated flag. + * Checks the user credentials by connecting to jamacloud and trying to pull project data. + * If the credentials are valid and the user is authenticated by Jama, then isAuth is set to true + * Sets the isAuthenticated flag. */ function isAuthenticated (req, res, next) { 'use strict'; @@ -30,13 +33,13 @@ function isAuthenticated (req, res, next) { let password = req.session.password; let teamName = req.session.teamName; let isAuth = false; + let isValid = validate(req); - if (username !== undefined && password !== undefined && teamName !== undefined) { - if (username !== '' && username.length <= 200 && - password.length >= 6 && password.length <= 200 && - teamName !== '') { - isAuth = true; - } + if (isValid) { + let url = 'http://' + username + ':' + password + '@' + teamName + '.jamacloud.com/rest/latest/projects'; + request({url: url, json: true}, function (error, response, body) { + isAuth = response.body.meta.status === 'OK' && !(error); + }); } req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag From 477b311fe28968dc3f2daf16dd0efb32c025260e Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Mon, 25 Jul 2016 11:26:12 -0700 Subject: [PATCH 09/15] added comments for readability. --- lib/auth.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/auth.js b/lib/auth.js index d86f7e4..e19957c 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -3,12 +3,25 @@ let pagination = require('./pagination'); let request = require('request'); // TODO: Separate responsibilities of authenticate into two parts: (1) log-in and (2) return projects + +/** + * Sets up the URL and passes it to Pagination + * @param username + * @param password + * @param teamName + * @returns {*} + */ function authenticate (username, password, teamName) { 'use strict'; let url = 'http://' + username + ':' + password + '@' + teamName + '.jamacloud.com/rest/latest/projects'; return pagination(url, 0, Number.MAX_SAFE_INTEGER); } +/** + * Validates that the username, password and teamname are not blank and meet the character length restrictions + * @param req + * @returns {boolean} + */ function validate (req) { 'use strict'; @@ -25,6 +38,9 @@ function validate (req) { * Checks the user credentials by connecting to jamacloud and trying to pull project data. * If the credentials are valid and the user is authenticated by Jama, then isAuth is set to true * Sets the isAuthenticated flag. + * @param req + * @param res + * @param next */ function isAuthenticated (req, res, next) { 'use strict'; From eeeb1e8038488299f0a53e4d85f6d00d8df6a88b Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Mon, 25 Jul 2016 14:08:17 -0700 Subject: [PATCH 10/15] added a separate function to authenticate on Jama's servers call jamaServerAuth --- lib/auth.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index e19957c..0399436 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -51,17 +51,20 @@ function isAuthenticated (req, res, next) { let isAuth = false; let isValid = validate(req); - if (isValid) { - let url = 'http://' + username + ':' + password + '@' + teamName + '.jamacloud.com/rest/latest/projects'; - request({url: url, json: true}, function (error, response, body) { - isAuth = response.body.meta.status === 'OK' && !(error); - }); + if (isValid && jamaServerAuth(username, password, teamName)) { + isAuth = true; } - req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag next(); // Continue logic flow as normal } +function jamaServerAuth (username, password, teamName) { + let url = 'http://' + username + ':' + password + '@' + teamName + '.jamacloud.com/rest/latest/'; + request({url: url, json: true}, function (error, response, body) { + return (response.body.meta.status === 'OK' && !(error)); + }); +} + module.exports = { authenticate: authenticate, validate: validate, From 8cceea0d214e39fde0ecf57477faee45f3b2063e Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Mon, 25 Jul 2016 14:09:23 -0700 Subject: [PATCH 11/15] Finished tests for isAuthenticated --- package.json | 3 ++ test/lib/auth.js | 89 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 71 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index e215e80..52e7b90 100644 --- a/package.json +++ b/package.json @@ -39,8 +39,11 @@ "istanbul": "^0.4.4", "mocha": "^2.5.3", "mocha-lcov-reporter": "^1.2.0", + "node-mocks-http": "^1.5.2", "proxyquire": "^1.7.10", "semistandard": "^8.0.0", + "sinon": "^1.17.4", + "sinon-chai": "^2.8.0", "snazzy": "^4.0.0", "watchify": "^3.7.0" } diff --git a/test/lib/auth.js b/test/lib/auth.js index ef931ba..ef71a82 100644 --- a/test/lib/auth.js +++ b/test/lib/auth.js @@ -5,8 +5,9 @@ let expect = chai.expect; let dirtyChai = require('dirty-chai'); let proxyquire = require('proxyquire'); let chaiAsPromised = require('chai-as-promised'); - let auth = require('../../lib/auth'); +let httpMocks = require('node-mocks-http'); +let sinon = require('sinon'); chai.use(dirtyChai); chai.use(chaiAsPromised); @@ -17,42 +18,50 @@ describe('Auth module', function () { { username: '', password: '', - teamName: '' + teamName: '', + description: 'all fields are empty' }, { username: 'dummy', password: '', - teamName: '' + teamName: '', + description: 'username is correct and password/teamname are empty' }, { username: '', password: 'dumber', - teamName: '' + teamName: '', + description: 'password is incorrect and username/teamname are empty' }, { username: '', password: '', - teamName: 'dummy' + teamName: 'dummy', + description: 'teamname is incorrect and password/username are empty' }, { username: 'dummy', password: 'password', - teamName: '' + teamName: '', + description: 'username is incorrect and password/teamname are empty' }, { username: '', password: 'password', - teamName: 'sevensource' + teamName: 'sevensource', + description: 'username is empty' }, { username: 'dummy', password: '', - teamName: 'sevensource' + teamName: 'sevensource', + description: 'username is incorrect and password is empty' }, { username: 'dummy', password: 'password', - teamName: 'sevensource' + teamName: 'sevensource', + description: 'all fields are valid' } ]; describe('validate function', function () { @@ -164,19 +173,57 @@ describe('Auth module', function () { }); describe('isAuthenticated function', () => { - let credentialsFixture = { body: {} }; + credentialFixtureCases.forEach(function (fixture) { + let credentialsFixture = { + session: {}, + body: {} + }; + credentialsFixture.session = fixture; + credentialsFixture.body = fixture; + auth = proxyquire('../../lib/auth', { + './validate': validateStub, + './jamaServerAuth': jamaServerAuthStub + }); + let callback = sinon.spy(); + let req = httpMocks.createRequest(); + let res = httpMocks.createResponse(); + req.session = {}; + req.body = {}; + req.session.username = fixture.username; + req.session.password = fixture.password; + req.session.teamName = fixture.teamName; + req.session.isAuthenticated = false; + req.body.username = fixture.username; + req.body.password = fixture.password; + req.body.teamName = fixture.teamName; + if (fixture.name === 'dummy' && fixture.password === 'password' && fixture.teamName === 'sevensource') { + it('should return true when ' + fixture.description, function (done) { + auth.isAuthenticated(req, res, callback); + expect(req.session.isAuthenticated).to.be.true(); + done(); + }); + } else { + it('should be false when ' + fixture.description, function (done) { + auth.isAuthenticated(req, res, callback); + expect(req.session.isAuthenticated).to.be.false(); + done(); + }); + } - it('should continue to specified route when user is authenticated', () => { - expect(res.session.username).to.equal('dummy'); - expect(res.session.password).to.equal('password'); - expect(res.session.teamName).to.equal('sevensource'); - }); + function validateStub (req) { + 'use strict'; + + let username = req.body.username; + let password = req.body.password; + let teamName = req.body.teamName; - it('should redirect to index page with session variable error code 401' + - 'using invalid credentials', function () { - // Place code here - expect(res).to.redirectTo('/'); - expect(res.session.error.id).to.equal(401); + return (username !== '' && username.length <= 200 && + password.length >= 6 && password.length <= 200 && + teamName !== ''); + } }); - }) + function jamaServerAuthStub (username, password, teamName) { + return (username === 'dummy' && password === 'password' && teamName === 'sevensource'); + } + }); }); From 7194735d0f2e9df16efa48adfe9db6fa5af10519 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Fri, 5 Aug 2016 10:54:46 -0700 Subject: [PATCH 12/15] Added function description and changed name of function that checks user credentials --- lib/auth.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/auth.js b/lib/auth.js index 0399436..fba6656 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -58,6 +58,12 @@ function isAuthenticated (req, res, next) { next(); // Continue logic flow as normal } +/** + * Checks the authentication status of credentials with Jama's servers + * @param username + * @param password + * @param teamName + */ function jamaServerAuth (username, password, teamName) { let url = 'http://' + username + ':' + password + '@' + teamName + '.jamacloud.com/rest/latest/'; request({url: url, json: true}, function (error, response, body) { From a6786871dfd8d36769be92e8aff162eabdff3e88 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Fri, 5 Aug 2016 11:03:13 -0700 Subject: [PATCH 13/15] Added some descriptions and parameter types to the function params --- lib/auth.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index fba6656..d8dbfd0 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -6,9 +6,9 @@ let request = require('request'); /** * Sets up the URL and passes it to Pagination - * @param username - * @param password - * @param teamName + * @param {string} username + * @param {string} password + * @param {string} teamName * @returns {*} */ function authenticate (username, password, teamName) { @@ -38,9 +38,9 @@ function validate (req) { * Checks the user credentials by connecting to jamacloud and trying to pull project data. * If the credentials are valid and the user is authenticated by Jama, then isAuth is set to true * Sets the isAuthenticated flag. - * @param req - * @param res - * @param next + * @param {object} req + * @param {object} res + * @param {object} next - The next function to be called after this one */ function isAuthenticated (req, res, next) { 'use strict'; @@ -51,7 +51,7 @@ function isAuthenticated (req, res, next) { let isAuth = false; let isValid = validate(req); - if (isValid && jamaServerAuth(username, password, teamName)) { + if (isValid && isServerAuthenticated(username, password, teamName)) { isAuth = true; } req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag @@ -60,11 +60,13 @@ function isAuthenticated (req, res, next) { /** * Checks the authentication status of credentials with Jama's servers - * @param username - * @param password - * @param teamName + * @param {string} username + * @param {string} password + * @param {string} teamName + * + * @returns {boolean} */ -function jamaServerAuth (username, password, teamName) { +function isServerAuthenticated (username, password, teamName) { let url = 'http://' + username + ':' + password + '@' + teamName + '.jamacloud.com/rest/latest/'; request({url: url, json: true}, function (error, response, body) { return (response.body.meta.status === 'OK' && !(error)); From 5877e91f28409cb5ebe341bad6cb166712f2d848 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Fri, 5 Aug 2016 11:55:04 -0700 Subject: [PATCH 14/15] Removed the validate function stub because it is not needed --- test/lib/auth.js | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/test/lib/auth.js b/test/lib/auth.js index ef71a82..a7987f3 100644 --- a/test/lib/auth.js +++ b/test/lib/auth.js @@ -181,12 +181,13 @@ describe('Auth module', function () { credentialsFixture.session = fixture; credentialsFixture.body = fixture; auth = proxyquire('../../lib/auth', { - './validate': validateStub, - './jamaServerAuth': jamaServerAuthStub + './isServerAuthenticated': isServerAuthenticatedStub }); + let callback = sinon.spy(); let req = httpMocks.createRequest(); let res = httpMocks.createResponse(); + req.session = {}; req.body = {}; req.session.username = fixture.username; @@ -196,6 +197,7 @@ describe('Auth module', function () { req.body.username = fixture.username; req.body.password = fixture.password; req.body.teamName = fixture.teamName; + if (fixture.name === 'dummy' && fixture.password === 'password' && fixture.teamName === 'sevensource') { it('should return true when ' + fixture.description, function (done) { auth.isAuthenticated(req, res, callback); @@ -209,20 +211,9 @@ describe('Auth module', function () { done(); }); } - - function validateStub (req) { - 'use strict'; - - let username = req.body.username; - let password = req.body.password; - let teamName = req.body.teamName; - - return (username !== '' && username.length <= 200 && - password.length >= 6 && password.length <= 200 && - teamName !== ''); - } }); - function jamaServerAuthStub (username, password, teamName) { + + function isServerAuthenticatedStub (username, password, teamName) { return (username === 'dummy' && password === 'password' && teamName === 'sevensource'); } }); From 13fda7a76128c5508ac8735493c4e581acf3be92 Mon Sep 17 00:00:00 2001 From: Ruben Piatnitsky Date: Tue, 16 Aug 2016 11:07:50 -0700 Subject: [PATCH 15/15] Removed superfluous comment --- lib/auth.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/auth.js b/lib/auth.js index d8dbfd0..7ebeb5a 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -55,7 +55,7 @@ function isAuthenticated (req, res, next) { isAuth = true; } req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag - next(); // Continue logic flow as normal + next(); } /**