From e04060b96a4e75ab2254ff181c4311dad5d07e51 Mon Sep 17 00:00:00 2001 From: Sergey Zakharchenko Date: Fri, 4 Dec 2020 14:53:48 +0000 Subject: [PATCH 1/4] Fix async tests Asynchronous tests need to run the 'done' function supplied as their argument; otherwise they are considered passing prematurely. --- test/digest-fetch-basic.js | 8 ++++++-- test/digest-fetch-rfc2069.js | 10 ++++++++-- test/digest-fetch-rfc2617.js | 15 ++++++++++++--- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/test/digest-fetch-basic.js b/test/digest-fetch-basic.js index a6c62a3..7333b13 100644 --- a/test/digest-fetch-basic.js +++ b/test/digest-fetch-basic.js @@ -13,18 +13,22 @@ var app = factory.getApp() describe('digest-fetch', function(){ - it('Test Basic Authentication', function() { + it('Test Basic Authentication', function(done) { var client = new DigestFetch('test', 'test', { basic: true }) const auth = client.addBasicAuth().headers.Authorization chai.request(app).get('/basic').set('Authorization', auth).then(res => { expect(res).to.have.status(200) + done() }) + .catch(done) }) - it('Test Basic Authentication with wrong credential', function() { + it('Test Basic Authentication with wrong credential', function(done) { var client = new DigestFetch('test', 'test-null', { basic: true }) const auth = client.addBasicAuth().headers.Authorization chai.request(app).get('/basic').set('Authorization', auth).then(res => { expect(res).to.have.status(401) + done() }) + .catch(done) }) }) diff --git a/test/digest-fetch-rfc2069.js b/test/digest-fetch-rfc2069.js index ff8d6db..6ca3b56 100644 --- a/test/digest-fetch-rfc2069.js +++ b/test/digest-fetch-rfc2069.js @@ -13,7 +13,7 @@ var app = factory.getApp() describe('digest-fetch', function(){ - it('Test RFC2069', function() { + it('Test RFC2069', function(done) { var client = new DigestFetch('test', 'test') chai.request(app).get('/auth').then(res => { expect(res).to.have.status(401) @@ -24,11 +24,14 @@ describe('digest-fetch', function(){ const auth = client.addAuth('/auth', { method: 'GET' }).headers.Authorization chai.request(app).get('/auth').set('Authorization', auth).then(res => { expect(res).to.have.status(200) + done() }) + .catch(done) }) + .catch(done) }) - it('Test RFC2069 with wrong credential', function() { + it('Test RFC2069 with wrong credential', function(done) { var client = new DigestFetch('test', 'test-null') chai.request(app).get('/auth').then(res => { res.should.have.status(401) @@ -39,7 +42,10 @@ describe('digest-fetch', function(){ const auth = client.addAuth('/auth', { method: 'GET' }).headers.Authorization chai.request(app).get('/auth').set('Authorization', auth).then(res => { expect(res).to.have.status(401) + done() }) + .catch(done) }) + .catch(done) }) }) diff --git a/test/digest-fetch-rfc2617.js b/test/digest-fetch-rfc2617.js index 747e596..030cf90 100644 --- a/test/digest-fetch-rfc2617.js +++ b/test/digest-fetch-rfc2617.js @@ -13,7 +13,7 @@ var app = factory.getApp('auth') describe('digest-fetch', function(){ - it('Test RFC2617', function() { + it('Test RFC2617', function(done) { var client = new DigestFetch('test', 'test') chai.request(app).get('/auth').then(res => { expect(res).to.have.status(401) @@ -24,11 +24,14 @@ describe('digest-fetch', function(){ const auth = client.addAuth('/auth', { method: 'GET' }).headers.Authorization chai.request(app).get('/auth').set('Authorization', auth).then(res => { expect(res).to.have.status(200) + done() }) + .catch(done) }) + .catch(done) }) - it('Test RFC2617 with precomputed hash', function() { + it('Test RFC2617 with precomputed hash', function(done) { var client = new DigestFetch('test', DigestFetch.computeHash('test', 'Users', 'test'), { precomputedHash: true }) chai.request(app).get('/auth').then(res => { expect(res).to.have.status(401) @@ -39,11 +42,14 @@ describe('digest-fetch', function(){ const auth = client.addAuth('/auth', { method: 'GET' }).headers.Authorization chai.request(app).get('/auth').set('Authorization', auth).then(res => { expect(res).to.have.status(200) + done() }) + .catch(done) }) + .catch(done) }) - it('Test RFC2617 with wrong credential', function() { + it('Test RFC2617 with wrong credential', function(done) { var client = new DigestFetch('test', 'test-null') chai.request(app).get('/auth').then(res => { expect(res).to.have.status(401) @@ -54,7 +60,10 @@ describe('digest-fetch', function(){ const auth = client.addAuth('/auth', { method: 'GET' }).headers.Authorization chai.request(app).get('/auth').set('Authorization', auth).then(res => { expect(res).to.have.status(401) + done() }) + .catch(done) }) + .catch(done) }) }) From 45ff1f50aa3cedc2a6052ffd3bccc7780a75c94f Mon Sep 17 00:00:00 2001 From: Sergey Zakharchenko Date: Fri, 4 Dec 2020 20:38:30 +0000 Subject: [PATCH 2/4] Fix arguments in parseAuth-related test parseAuth expects a full WWW-Authenticate header value which includes the scheme, so it makes sense to provide it in tests. --- test/digest-fetch.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/digest-fetch.js b/test/digest-fetch.js index 5804f2b..50b62f2 100644 --- a/test/digest-fetch.js +++ b/test/digest-fetch.js @@ -24,9 +24,9 @@ describe('digest-fetch', function(){ assert.equal(DigestFetch.parse('a="v2",', 'a'), 'v2') assert.equal(DigestFetch.parse('a="v1,v2"', 'a'), 'v1,v2') const client = new DigestFetch("", "") - client.parseAuth('qop=auth-int,realm=test') + client.parseAuth('Digest qop=auth-int,realm=test',1) assert.equal(client.digest.realm, "test") - client.parseAuth('qop="auth",realm="v1 v2"') + client.parseAuth('Digest qop="auth",realm="v1 v2"') assert.equal(client.digest.realm, "v1 v2") }) From 28f2fc74d435a34ef026e9e380c2c4f34785db18 Mon Sep 17 00:00:00 2001 From: Sergey Zakharchenko Date: Fri, 4 Dec 2020 20:59:13 +0000 Subject: [PATCH 3/4] Add tests for using Digest auth if offered both Clients should choose the safer alternative among the supported auth methods, which, in the case both Basic and Digest are offered, is Digest. --- test/digest-fetch-basic.js | 18 +++++++++++++++++ test/digest-fetch-rfc2617.js | 38 ++++++++++++++++++++++++++++++++++++ test/test-server.js | 12 ++++++++++++ 3 files changed, 68 insertions(+) diff --git a/test/digest-fetch-basic.js b/test/digest-fetch-basic.js index 7333b13..0b7bc21 100644 --- a/test/digest-fetch-basic.js +++ b/test/digest-fetch-basic.js @@ -31,4 +31,22 @@ describe('digest-fetch', function(){ }) .catch(done) }) + it('Test Basic Authentication with basic-and-digest', function(done) { + var client = new DigestFetch('test', 'test', { basic: true }) + const auth = client.addBasicAuth().headers.Authorization + chai.request(app).get('/basic-and-digest').set('Authorization', auth).then(res => { + expect(res).to.have.status(200) + done() + }) + .catch(done) + }) + it('Test Basic Authentication with digest-and-basic', function(done) { + var client = new DigestFetch('test', 'test', { basic: true }) + const auth = client.addBasicAuth().headers.Authorization + chai.request(app).get('/digest-and-basic').set('Authorization', auth).then(res => { + expect(res).to.have.status(200) + done() + }) + .catch(done) + }) }) diff --git a/test/digest-fetch-rfc2617.js b/test/digest-fetch-rfc2617.js index 030cf90..f3a4ab3 100644 --- a/test/digest-fetch-rfc2617.js +++ b/test/digest-fetch-rfc2617.js @@ -66,4 +66,42 @@ describe('digest-fetch', function(){ }) .catch(done) }) + + it('Test RFC2617 with basic-and-digest', function(done) { + var client = new DigestFetch('test', 'test') + chai.request(app).get('/basic-and-digest').then(res => { + expect(res).to.have.status(401) + client.lastAuth = res.res.headers['www-authenticate'] + }) + .then(() => { + client.parseAuth(client.lastAuth) + const auth = client.addAuth('/basic-and-digest', { method: 'GET' }).headers.Authorization + expect(auth).to.match(/^Digest /) + chai.request(app).get('/basic-and-digest').set('Authorization', auth).then(res => { + expect(res).to.have.status(200) + done() + }) + .catch(done) + }) + .catch(done) + }) + + it('Test RFC2617 with digest-and-basic', function(done) { + var client = new DigestFetch('test', 'test') + chai.request(app).get('/digest-and-basic').then(res => { + expect(res).to.have.status(401) + client.lastAuth = res.res.headers['www-authenticate'] + }) + .then(() => { + client.parseAuth(client.lastAuth) + const auth = client.addAuth('/digest-and-basic', { method: 'GET' }).headers.Authorization + expect(auth).to.match(/^Digest /) + chai.request(app).get('/digest-and-basic').set('Authorization', auth).then(res => { + expect(res).to.have.status(200) + done() + }) + .catch(done) + }) + .catch(done) + }) }) diff --git a/test/test-server.js b/test/test-server.js index 611195f..a9d4684 100644 --- a/test/test-server.js +++ b/test/test-server.js @@ -53,6 +53,18 @@ module.exports = { getApp(method) { function(req, res) { res.json(req.user); }); + // http basic and digest authentication + app.get('/basic-and-digest', + passport.authenticate(['basic', 'digest'], { session: false }), + function(req, res) { + res.json(req.user); + }); + // http digest and basic authentication + app.get('/digest-and-basic', + passport.authenticate(['digest','basic'], { session: false }), + function(req, res) { + res.json(req.user); + }); // app.use("/static", express.static(path.join(__dirname, './static'))); From 1144eadfbfa91222924128b59b90598510225bfc Mon Sep 17 00:00:00 2001 From: Sergey Zakharchenko Date: Fri, 4 Dec 2020 21:02:40 +0000 Subject: [PATCH 4/4] Fix multiple-method WWW-Authenticate parsing Split WWW-Authenticate by challenge and handle digest-related properties only for digest method. --- digest-fetch-src.js | 47 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/digest-fetch-src.js b/digest-fetch-src.js index 85b5e20..26d623e 100644 --- a/digest-fetch-src.js +++ b/digest-fetch-src.js @@ -12,6 +12,14 @@ const base64 = require('base-64') const supported_algorithms = ['MD5', 'MD5-sess'] +// challenge = auth-scheme 1*SP 1#auth-param + +const tokenRegexp = '[!#-\'*+-.0-9A-Z^_`a-z~]+' +const schemeRegexp = tokenRegexp +const paramRegexp = tokenRegexp+'='+'('+tokenRegexp+'|"([^"\\\\]|\\\\.)*")' + +const challengeRegexp = new RegExp('^(('+schemeRegexp+') +'+paramRegexp+'( *, *'+paramRegexp+')*) *(, *'+schemeRegexp+' .*)?$') + const parse = (raw, field, trim=true) => { const regex = new RegExp(`${field}=("[^"]*"|[^,]*)`, "i") const match = regex.exec(raw) @@ -117,7 +125,7 @@ class DigestClient { const opaqueString = this.digest.opaque !== null ? `opaque="${this.digest.opaque}",` : '' const qopString = this.digest.qop ? `qop="${this.digest.qop}",` : '' - const digest = `${this.digest.scheme} username="${this.user}",realm="${this.digest.realm}",\ + const digest = `Digest username="${this.user}",realm="${this.digest.realm}",\ nonce="${this.digest.nonce}",uri="${uri}",${opaqueString}${qopString}\ algorithm="${this.digest.algorithm}",response="${response}",nc=${ncString},cnonce="${this.digest.cnonce}"` options.headers = options.headers || {} @@ -140,19 +148,38 @@ algorithm="${this.digest.algorithm}",response="${response}",nc=${ncString},cnonc } this.hasAuth = true - - this.digest.scheme = h.split(/\s/)[0] + while (true) { + const challengeMatch = h.match(challengeRegexp) + if (!challengeMatch) { + break + } + const challenge = challengeMatch[1] + const scheme = challengeMatch[2] + + if (scheme.match(/^Digest$/i)) { - this.digest.realm = (parse(h, 'realm', false) || '').replace(/["]/g, '') + this.digest.realm = (parse(challenge, 'realm', false) || '').replace(/["]/g, '') - this.digest.qop = this.parseQop(h) + this.digest.qop = this.parseQop(challenge) - this.digest.opaque = parse(h, 'opaque') - - this.digest.nonce = parse(h, 'nonce') || '' + this.digest.opaque = parse(challenge, 'opaque') - this.digest.cnonce = this.makeNonce() - this.digest.nc++ + this.digest.nonce = parse(challenge, 'nonce') || '' + + this.digest.cnonce = this.makeNonce() + this.digest.nc++ + } + h = h.substr(challenge.length) + if (!h) { + break + } + const comma = h.match("^( *, *).*") + if (!comma) { + // TODO: parse error + break + } + h = h.substr(comma[1].length) + } } parseQop (rawAuth) {