-
Notifications
You must be signed in to change notification settings - Fork 7
Unauthorized access #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Unauthorized access #132
Changes from all commits
8e4dcab
70c093a
f60cbea
f2e6254
7e758e6
ccd780b
a20a121
bda50c9
0ae8a4b
477b311
eeeb1e8
8cceea0
7194735
a678687
5877e91
d741140
13fda7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,27 @@ | ||
| '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 | ||
|
|
||
| /** | ||
| * Sets up the URL and passes it to Pagination | ||
| * @param {string} username | ||
| * @param {string} password | ||
| * @param {string} 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'; | ||
|
|
||
|
|
@@ -20,7 +34,47 @@ function validate (req) { | |
| teamName !== ''); | ||
| } | ||
|
|
||
| /** | ||
| * 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 {object} req | ||
| * @param {object} res | ||
| * @param {object} next - The next function to be called after this one | ||
| */ | ||
| function isAuthenticated (req, res, next) { | ||
| 'use strict'; | ||
|
|
||
| let username = req.session.username; | ||
| let password = req.session.password; | ||
| let teamName = req.session.teamName; | ||
| let isAuth = false; | ||
| let isValid = validate(req); | ||
|
|
||
| if (isValid && isServerAuthenticated(username, password, teamName)) { | ||
| isAuth = true; | ||
| } | ||
| req.session.isAuthenticated = isAuth; // Set the isAuthenticate flag | ||
| next(); | ||
| } | ||
|
|
||
| /** | ||
| * Checks the authentication status of credentials with Jama's servers | ||
| * @param {string} username | ||
| * @param {string} password | ||
| * @param {string} teamName | ||
| * | ||
| * @returns {boolean} | ||
| */ | ||
| function isServerAuthenticated (username, password, teamName) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function isn't necessary. As I said before, once you log in, our application stores the username and password in the session. Just check to see if they aren't falsey. |
||
| 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 | ||
| validate: validate, | ||
| isAuthenticated: isAuthenticated | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,8 +41,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", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sinon-chai is unused, remove it. |
||
| "snazzy": "^4.0.0", | ||
| "supertest": "^2.0.0", | ||
| "watchify": "^3.7.0" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 () { | ||
|
|
@@ -162,4 +171,50 @@ describe('Auth module', function () { | |
| expect(auth.authenticate(username, password, teamName)).to.be.rejected(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isAuthenticated function', () => { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests can be changed accordingly to the comments above |
||
| credentialFixtureCases.forEach(function (fixture) { | ||
| let credentialsFixture = { | ||
| session: {}, | ||
| body: {} | ||
| }; | ||
| credentialsFixture.session = fixture; | ||
| credentialsFixture.body = fixture; | ||
| auth = proxyquire('../../lib/auth', { | ||
| './isServerAuthenticated': isServerAuthenticatedStub | ||
| }); | ||
|
|
||
| let callback = sinon.spy(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The callback (a stub for next, right?) isn't checked after. Add |
||
| 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(); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| function isServerAuthenticatedStub (username, password, teamName) { | ||
| return (username === 'dummy' && password === 'password' && teamName === 'sevensource'); | ||
| } | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just need to check if the session fields aren't falsey here, you don't have to make a request to jama. teamName is stored in .env in so you don't need to check the req.session.teamName field for that.
Then you can store the actual result in the req object so we can check it in the routes.