diff --git a/CHANGELOG.yaml b/CHANGELOG.yaml index 71da75ca9..7375b5e9a 100644 --- a/CHANGELOG.yaml +++ b/CHANGELOG.yaml @@ -1,4 +1,8 @@ master: + fixed bugs: + - >- + GH-945 Fixed a bug where NTLM requests were failing for `iterationCount` + more that 2 chores: - GH-943 Added `nyc` and `codecov` for code coverage checks diff --git a/lib/authorizer/ntlm.js b/lib/authorizer/ntlm.js index 5ffdc538e..4dc08e803 100644 --- a/lib/authorizer/ntlm.js +++ b/lib/authorizer/ntlm.js @@ -128,7 +128,10 @@ module.exports = { * @param {AuthHandlerInterface~authPreHookCallback} done */ pre: function (auth, done) { - !auth.get(STATE) && auth.set(STATE, STATES.INITIALIZED); + if (!auth.get(STATE)) { + auth.set(STATE, STATES.INITIALIZED); + auth.set(NTLM_HEADER, undefined); + } done(null, true); }, @@ -154,10 +157,18 @@ module.exports = { challengeMessage, // type 2 authenticateMessage, // type 3 ntlmType2Header, - parsedParameters; + parsedParameters, + + // resets the state and NTLM header and exits replay loop + resetStateAndStop = function (err) { + auth.set(STATE, STATES.INITIALIZED); + auth.set(NTLM_HEADER, undefined); + + return done(err || null, true); + }; if (response.code !== 401 && response.code !== 403) { - return done(null, true); + return resetStateAndStop(); } // we try to extract domain from username if not specified. @@ -172,7 +183,7 @@ module.exports = { // Nothing to do if the server does not ask us for auth in the first place. if (!(response.headers.has(WWW_AUTHENTICATE, NTLM) || response.headers.has(WWW_AUTHENTICATE, NEGOTIATE))) { - return done(null, true); + return resetStateAndStop(); } // Create a type 1 message to send to the server @@ -202,13 +213,13 @@ module.exports = { }); if (!ntlmType2Header) { - return done(new Error('ntlm: server did not send NTLM type 2 message')); + return resetStateAndStop(new Error('ntlm: server did not send NTLM type 2 message')); } challengeMessage = ntlmUtil.parseType2Message(ntlmType2Header.valueOf(), _.noop); if (!challengeMessage) { - return done(new Error('ntlm: server did not correctly process authentication request')); + return resetStateAndStop(new Error('ntlm: server did not correctly process authentication request')); } authenticateMessage = ntlmUtil.createType3Message(challengeMessage, { @@ -227,11 +238,11 @@ module.exports = { } else if (state === STATES.T3_MSG_CREATED) { // Means we have tried to authenticate, so we should stop here without worrying about anything - return done(null, true); + return resetStateAndStop(); } // We are in an undefined state - return done(null, true); + return resetStateAndStop(); }, /** diff --git a/lib/runner/extensions/item.command.js b/lib/runner/extensions/item.command.js index 292ffc3e3..5820b280a 100644 --- a/lib/runner/extensions/item.command.js +++ b/lib/runner/extensions/item.command.js @@ -188,7 +188,8 @@ module.exports = { var request = result.request, response = result.response, - cookies = result.cookies; + cookies = result.cookies, + auth = result.item.getAuth(); if ((stopOnError || stopOnFailure) && requestError) { this.triggers.item(null, coords, item); // @todo - should this trigger receive error? @@ -211,6 +212,11 @@ module.exports = { // set cookies for this transaction cookies && (ctxTemplate.cookies = cookies); + // update the item auth to prevent loss of auth state + // @note this is a hack because the auth architecture is messy, and there is some confusion + // between `originalItem` and `item`. + auth && (item.auth = auth); + // the context template also has a test object to store assertions ctxTemplate.tests = {}; // @todo remove diff --git a/test/fixtures/server.js b/test/fixtures/server.js index da26bd443..6645efc78 100644 --- a/test/fixtures/server.js +++ b/test/fixtures/server.js @@ -537,6 +537,8 @@ function createNTLMServer (options) { domain = options.domain || '', workstation = options.workstation || '', + challenged = false, + type1Message = ntlmUtils.createType1Message({ domain, workstation @@ -559,6 +561,7 @@ function createNTLMServer (options) { // sure that runtime can handle it. 'www-authenticate': [type2Message, 'Negotiate'] }); + challenged = true; options.debug && console.info('401: got type1 message'); } @@ -566,7 +569,7 @@ function createNTLMServer (options) { // successful auth // @note we don't check if the username and password are correct // because I don't know how. - else if (authHeaders && authHeaders.startsWith(type3Message.slice(0, 100))) { + else if (challenged && authHeaders && authHeaders.startsWith(type3Message.slice(0, 100))) { res.writeHead(200); options.debug && console.info('200: got type3 message'); diff --git a/test/integration/auth-methods/ntlm.test.js b/test/integration/auth-methods/ntlm.test.js index 86447ab63..aad0e5ab5 100644 --- a/test/integration/auth-methods/ntlm.test.js +++ b/test/integration/auth-methods/ntlm.test.js @@ -479,4 +479,330 @@ describe('NTLM', function () { expect(response).to.have.property('code', 401); }); }); + + describe('with NTLM auth set at collection level', function () { + before(function (done) { + var localRunOptions = { + collection: { + item: { + name: 'NTLM Sample Request', + request: { + url: ntlmServerURL + } + }, + auth: { + type: 'ntlm', + ntlm: { + username: USERNAME, + password: PASSWORD, + domain: DOMAIN, + workstation: WORKSTATION + } + } + } + }; + + // perform the collection run + this.run(localRunOptions, function (err, results) { + testrun = results; + done(err); + }); + }); + + it('should have completed the run', function () { + expect(testrun).to.be.ok; + expect(testrun).to.nested.include({ + 'done.callCount': 1 + }); + + var err = testrun.request.firstCall.args[0]; + + err && console.error(err.stack); + expect(err).to.be.null; + + expect(testrun).to.nested.include({ + 'start.callCount': 1 + }); + }); + + it('should have sent the request thrice', function () { + expect(testrun).to.nested.include({ + 'request.callCount': 3 + }); + + var response = testrun.request.lastCall.args[2]; + + expect(response).to.have.property('code', 200); + }); + }); + + describe('with NTLM auth set at folder level', function () { + before(function (done) { + var localRunOptions = { + collection: { + item: { + name: 'NTLM folder', + item: { + name: 'NTLM Sample Request', + request: { + url: ntlmServerURL + } + }, + auth: { + type: 'ntlm', + ntlm: { + username: USERNAME, + password: PASSWORD, + domain: DOMAIN, + workstation: WORKSTATION + } + } + } + } + }; + + // perform the collection run + this.run(localRunOptions, function (err, results) { + testrun = results; + done(err); + }); + }); + + it('should have completed the run', function () { + expect(testrun).to.be.ok; + expect(testrun).to.nested.include({ + 'done.callCount': 1 + }); + + var err = testrun.request.firstCall.args[0]; + + err && console.error(err.stack); + expect(err).to.be.null; + + expect(testrun).to.nested.include({ + 'start.callCount': 1 + }); + }); + + it('should have sent the request thrice', function () { + expect(testrun).to.nested.include({ + 'request.callCount': 3 + }); + + var response = testrun.request.lastCall.args[2]; + + expect(response).to.have.property('code', 200); + }); + }); + + describe('with NTLM auth set at request level and 5 iterations', function () { + before(function (done) { + var localRunOptions = { + collection: { + item: { + name: 'NTLM Sample Request', + request: { + url: ntlmServerURL + } + }, + auth: { + type: 'ntlm', + ntlm: { + username: USERNAME, + password: PASSWORD, + domain: DOMAIN, + workstation: WORKSTATION + } + } + }, + iterationCount: 5 + }; + + // perform the collection run + this.run(localRunOptions, function (err, results) { + testrun = results; + done(err); + }); + }); + + it('should have completed the run', function () { + expect(testrun).to.be.ok; + expect(testrun).to.nested.include({ + 'done.callCount': 1 + }); + + var err = testrun.request.firstCall.args[0]; + + err && console.error(err.stack); + expect(err).to.be.null; + + expect(testrun).to.nested.include({ + 'start.callCount': 1 + }); + }); + + it('should have sent the request 3 * 5 = 15 times', function () { + expect(testrun).to.nested.include({ + 'request.callCount': 15 + }); + + var response0 = testrun.request.getCall(0).args[2], + response1 = testrun.request.getCall(1).args[2], + response2 = testrun.request.getCall(2).args[2], + response5 = testrun.request.getCall(5).args[2], + response8 = testrun.request.getCall(8).args[2], + response11 = testrun.request.getCall(11).args[2], + response14 = testrun.request.getCall(14).args[2]; + + expect(response0, 'iteration 1, request 1').to.have.property('code', 401); + expect(response1, 'iteration 1, request 2').to.have.property('code', 401); + expect(response2, 'iteration 1, request 3').to.have.property('code', 200); + expect(response5, 'iteration 2, request 3').to.have.property('code', 200); + expect(response8, 'iteration 3, request 3').to.have.property('code', 200); + expect(response11, 'iteration 4, request 3').to.have.property('code', 200); + expect(response14, 'iteration 5, request 3').to.have.property('code', 200); + }); + }); + + + describe('with NTLM auth set at collection level and 5 iterations', function () { + before(function (done) { + var localRunOptions = { + collection: { + item: { + name: 'NTLM Sample Request', + request: { + url: ntlmServerURL + } + }, + auth: { + type: 'ntlm', + ntlm: { + username: USERNAME, + password: PASSWORD, + domain: DOMAIN, + workstation: WORKSTATION + } + } + }, + iterationCount: 5 + }; + + // perform the collection run + this.run(localRunOptions, function (err, results) { + testrun = results; + done(err); + }); + }); + + it('should have completed the run', function () { + expect(testrun).to.be.ok; + expect(testrun).to.nested.include({ + 'done.callCount': 1 + }); + + var err = testrun.request.firstCall.args[0]; + + err && console.error(err.stack); + expect(err).to.be.null; + + expect(testrun).to.nested.include({ + 'start.callCount': 1 + }); + }); + + it('should have sent the request 3 * 5 = 15 times', function () { + expect(testrun).to.nested.include({ + 'request.callCount': 15 + }); + + var response0 = testrun.request.getCall(0).args[2], + response1 = testrun.request.getCall(1).args[2], + response2 = testrun.request.getCall(2).args[2], + response5 = testrun.request.getCall(5).args[2], + response8 = testrun.request.getCall(8).args[2], + response11 = testrun.request.getCall(11).args[2], + response14 = testrun.request.getCall(14).args[2]; + + expect(response0, 'iteration 1, request 1').to.have.property('code', 401); + expect(response1, 'iteration 1, request 2').to.have.property('code', 401); + expect(response2, 'iteration 1, request 3').to.have.property('code', 200); + expect(response5, 'iteration 2, request 3').to.have.property('code', 200); + expect(response8, 'iteration 3, request 3').to.have.property('code', 200); + expect(response11, 'iteration 4, request 3').to.have.property('code', 200); + expect(response14, 'iteration 5, request 3').to.have.property('code', 200); + }); + }); + + describe('with NTLM auth set at folder level and 5 iterations', function () { + before(function (done) { + var localRunOptions = { + collection: { + item: { + name: 'NTLM folder', + item: { + name: 'NTLM Sample Request', + request: { + url: ntlmServerURL + } + }, + auth: { + type: 'ntlm', + ntlm: { + username: USERNAME, + password: PASSWORD, + domain: DOMAIN, + workstation: WORKSTATION + } + } + } + }, + iterationCount: 5 + }; + + // perform the collection run + this.run(localRunOptions, function (err, results) { + testrun = results; + done(err); + }); + }); + + it('should have completed the run', function () { + expect(testrun).to.be.ok; + expect(testrun).to.nested.include({ + 'done.callCount': 1 + }); + + var err = testrun.request.firstCall.args[0]; + + err && console.error(err.stack); + expect(err).to.be.null; + + expect(testrun).to.nested.include({ + 'start.callCount': 1 + }); + }); + + it('should have sent the request 3 * 5 = 15 times', function () { + expect(testrun).to.nested.include({ + 'request.callCount': 15 + }); + + var response0 = testrun.request.getCall(0).args[2], + response1 = testrun.request.getCall(1).args[2], + response2 = testrun.request.getCall(2).args[2], + response5 = testrun.request.getCall(5).args[2], + response8 = testrun.request.getCall(8).args[2], + response11 = testrun.request.getCall(11).args[2], + response14 = testrun.request.getCall(14).args[2]; + + expect(response0, 'iteration 1, request 1').to.have.property('code', 401); + expect(response1, 'iteration 1, request 2').to.have.property('code', 401); + expect(response2, 'iteration 1, request 3').to.have.property('code', 200); + expect(response5, 'iteration 2, request 3').to.have.property('code', 200); + expect(response8, 'iteration 3, request 3').to.have.property('code', 200); + expect(response11, 'iteration 4, request 3').to.have.property('code', 200); + expect(response14, 'iteration 5, request 3').to.have.property('code', 200); + }); + }); });