From 8815e8b24ed06751602bbe3da66c9f2fa3ae4b37 Mon Sep 17 00:00:00 2001 From: Utkarsh Maheshwari Date: Thu, 19 Dec 2019 12:40:28 +0530 Subject: [PATCH 1/5] Add tests for NTLM at folder and request level --- test/integration/auth-methods/ntlm.test.js | 326 +++++++++++++++++++++ 1 file changed, 326 insertions(+) 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); + }); + }); }); From 52451f82f6bd809700de574e70925ca9a2f17235 Mon Sep 17 00:00:00 2001 From: Utkarsh Maheshwari Date: Wed, 18 Dec 2019 16:41:25 +0530 Subject: [PATCH 2/5] Fix NTLM auth state when iterations are more than 3 --- lib/authorizer/ntlm.js | 8 +++++++- lib/runner/extensions/item.command.js | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/authorizer/ntlm.js b/lib/authorizer/ntlm.js index 5ffdc538e..e7b60ecca 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); }, @@ -157,6 +160,9 @@ module.exports = { parsedParameters; if (response.code !== 401 && response.code !== 403) { + auth.set(STATE, STATES.INITIALIZED); + auth.set(NTLM_HEADER, undefined); + return done(null, true); } 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 From 36533b3082281496ef0340ffe8aaca96f59e3d6b Mon Sep 17 00:00:00 2001 From: Utkarsh Maheshwari Date: Thu, 19 Dec 2019 12:47:13 +0530 Subject: [PATCH 3/5] Update CHANGELOG --- CHANGELOG.yaml | 4 ++++ 1 file changed, 4 insertions(+) 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 From 58fe97be0e8280db8b04d6989d6ed21837d9060a Mon Sep 17 00:00:00 2001 From: Utkarsh Maheshwari Date: Thu, 19 Dec 2019 16:39:48 +0530 Subject: [PATCH 4/5] Reset NTLM auth state on every exit from replay loop --- lib/authorizer/ntlm.js | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/authorizer/ntlm.js b/lib/authorizer/ntlm.js index e7b60ecca..4dc08e803 100644 --- a/lib/authorizer/ntlm.js +++ b/lib/authorizer/ntlm.js @@ -157,13 +157,18 @@ module.exports = { challengeMessage, // type 2 authenticateMessage, // type 3 ntlmType2Header, - parsedParameters; + parsedParameters, - if (response.code !== 401 && response.code !== 403) { - auth.set(STATE, STATES.INITIALIZED); - auth.set(NTLM_HEADER, undefined); + // 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(null, true); + return done(err || null, true); + }; + + if (response.code !== 401 && response.code !== 403) { + return resetStateAndStop(); } // we try to extract domain from username if not specified. @@ -178,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 @@ -208,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, { @@ -233,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(); }, /** From 87179ca7f46c5d731b4e45b57171ce29f9af068f Mon Sep 17 00:00:00 2001 From: Utkarsh Maheshwari Date: Mon, 23 Dec 2019 18:19:45 +0530 Subject: [PATCH 5/5] fixup! Add tests for NTLM at folder and request level --- test/fixtures/server.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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');