From f65800306ca7daca0351f7629d62e1b51427d521 Mon Sep 17 00:00:00 2001 From: Diana Barsan Date: Mon, 12 Feb 2024 21:08:42 +0700 Subject: [PATCH 1/6] handle concurrent session bearing requests --- src/index.js | 29 ++++++++++++++++++----------- test/integration/index.spec.js | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/index.js b/src/index.js index 407a7b3..eff350d 100644 --- a/src/index.js +++ b/src/index.js @@ -33,9 +33,9 @@ const parseCookie = (response) => { }; }; -const getExistingSession = (db) => { +const getExistingSession = async (db) => { const urlString = getSessionKey(db); - if (sessions[urlString] && !isExpired(sessions[urlString])) { + if (sessions[urlString] && !isExpired(await sessions[urlString])) { return sessions[urlString]; } }; @@ -60,14 +60,15 @@ const authenticate = async (db) => { const body = JSON.stringify({ name: db.credentials.username, password: db.credentials.password}); const response = await db.originalFetch(url.toString(), { method: 'POST', headers, body }); - updateSession(db, response); + return updateSession(db, response); }; const updateSession = (db, response) => { const session = parseCookie(response); if (session) { - const url = getSessionKey(db); - sessions[url] = session; + const sessionKey = getSessionKey(db); + sessions[sessionKey] = Promise.resolve(session); + return session; } }; @@ -76,8 +77,18 @@ const invalidateSession = db => { delete sessions[url]; }; +const getSession = async (db) => { + const session = await getExistingSession(db); + + if (session) { + return session; + } + + return await authenticate(db); +}; + const isExpired = (session) => { - return Date.now() > session.expires; + return Date.now() > session?.expires; }; const extractAuth = (opts) => { @@ -108,11 +119,7 @@ function wrapAdapter (PouchDB, HttpPouch) { db.originalFetch = db.fetch || PouchDB.fetch; db.fetch = async (url, opts = {}) => { - let session = getExistingSession(db); - if (!session) { - await authenticate(db); - session = getExistingSession(db); - } + const session = await getSession(db); if (session) { opts.headers = opts.headers || new Headers(); diff --git a/test/integration/index.spec.js b/test/integration/index.spec.js index 4a6ae69..71f50b2 100644 --- a/test/integration/index.spec.js +++ b/test/integration/index.spec.js @@ -143,5 +143,23 @@ describe(`integration with ${authType}`, function () { const logs = await collectLogs(1000); expect(utils.getSessionRequests(logs).length).to.equal(1); }); + + it('should only request session once for concurrent requests', async () => { + const auth = { username: tempAdminName, password: 'new_password' }; + await utils.createAdmin(auth.username, auth.password); + + const collectLogs = await utils.getDockerContainerLogs(); + tempDb = getDb(tempDbName, auth, authType, false); + await Promise.all([ + tempDb.allDocs(), + tempDb.allDocs(), + tempDb.allDocs(), + tempDb.allDocs(), + tempDb.allDocs(), + ]); + + const logs = await collectLogs(); + expect(utils.getSessionRequests(logs).length).to.equal(1); + }); }); From fc39ea9007ad4dfce050341f90f95e104cc2ecf4 Mon Sep 17 00:00:00 2001 From: Diana Barsan Date: Tue, 13 Feb 2024 13:59:15 +0700 Subject: [PATCH 2/6] handle concurrent session bearing requests --- src/index.js | 32 +++++++++++++--------- test/integration/index.spec.js | 3 ++- test/unit/index.spec.js | 49 ++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/index.js b/src/index.js index eff350d..d23eb0f 100644 --- a/src/index.js +++ b/src/index.js @@ -33,13 +33,6 @@ const parseCookie = (response) => { }; }; -const getExistingSession = async (db) => { - const urlString = getSessionKey(db); - if (sessions[urlString] && !isExpired(await sessions[urlString])) { - return sessions[urlString]; - } -}; - const getSessionKey = (db) => { const sessionUrl = getSessionUrl(db); return `${db.credentials.username}:${db.credentials.password}:${sessionUrl}`; @@ -73,18 +66,20 @@ const updateSession = (db, response) => { }; const invalidateSession = db => { - const url = getSessionKey(db); - delete sessions[url]; + const sessionKey = getSessionKey(db); + delete sessions[sessionKey]; }; const getSession = async (db) => { - const session = await getExistingSession(db); + const sessionKey = getSessionKey(db); + const session = sessions[sessionKey]; if (session) { return session; } - return await authenticate(db); + sessions[sessionKey] = authenticate(db); + return sessions[sessionKey]; }; const isExpired = (session) => { @@ -107,6 +102,19 @@ const extractAuth = (opts) => { }; }; +const validateSession = (db, session) => { + if (!session) { + return; + } + + if (isExpired(session)) { + invalidateSession(db); + return getSession(db); + } + + return session; +}; + // eslint-disable-next-line func-style function wrapAdapter (PouchDB, HttpPouch) { // eslint-disable-next-line func-style @@ -119,7 +127,7 @@ function wrapAdapter (PouchDB, HttpPouch) { db.originalFetch = db.fetch || PouchDB.fetch; db.fetch = async (url, opts = {}) => { - const session = await getSession(db); + const session = await validateSession(db, await getSession(db)); if (session) { opts.headers = opts.headers || new Headers(); diff --git a/test/integration/index.spec.js b/test/integration/index.spec.js index 71f50b2..5d16201 100644 --- a/test/integration/index.spec.js +++ b/test/integration/index.spec.js @@ -147,9 +147,10 @@ describe(`integration with ${authType}`, function () { it('should only request session once for concurrent requests', async () => { const auth = { username: tempAdminName, password: 'new_password' }; await utils.createAdmin(auth.username, auth.password); + await utils.createDb(tempDbName); + tempDb = getDb(tempDbName, auth, authType); const collectLogs = await utils.getDockerContainerLogs(); - tempDb = getDb(tempDbName, auth, authType, false); await Promise.all([ tempDb.allDocs(), tempDb.allDocs(), diff --git a/test/unit/index.spec.js b/test/unit/index.spec.js index ef2edf6..057b6b6 100644 --- a/test/unit/index.spec.js +++ b/test/unit/index.spec.js @@ -289,6 +289,55 @@ describe('Pouchdb Session authentication plugin', () => { { headers: new Headers({ 'Cookie': 'AuthSession=user2session' }) } ]); }); + + it('should only request session once for concurrent requests', async () => { + db = { name: 'http://admin:pass@localhost:5984/mydb' }; + plugin(PoudhDb); + PoudhDb.adapters.http(db); + + fetch.resolves({ ok: true, status: 200 }); + fetch.withArgs('http://admin:pass@localhost:5984/_session').resolves({ + ok: true, + status: 200, + headers: new Headers({ 'set-cookie': getSession('theonetruesession') }) + }); + + await Promise.all([ + db.fetch('randomUrl1'), + db.fetch('randomUrl2'), + db.fetch('randomUrl3'), + db.fetch('randomUrl4'), + ]); + + expect(fetch.callCount).to.equal(5); + expect(fetch.args[0]).to.deep.equal([ + 'http://admin:pass@localhost:5984/_session', + { + method: 'POST', + headers: new Headers({ + 'Content-Type': 'application/json', + 'Accept': 'application/json', + }), + body: JSON.stringify({ name: 'admin', password: 'pass' }), + } + ]); + expect(fetch.args[1]).to.deep.equal([ + 'randomUrl1', + { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } + ]); + expect(fetch.args[2]).to.deep.equal([ + 'randomUrl2', + { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } + ]); + expect(fetch.args[3]).to.deep.equal([ + 'randomUrl3', + { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } + ]); + expect(fetch.args[4]).to.deep.equal([ + 'randomUrl4', + { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } + ]); + }); it('should update the session if server responds with new cookie', async () => { db = { name: 'http://admin:pass@localhost:5984/mydb' }; From ca0f840caaf0624de403ac9d9507b7826685aec1 Mon Sep 17 00:00:00 2001 From: Diana Barsan Date: Tue, 13 Feb 2024 15:38:49 +0700 Subject: [PATCH 3/6] no need to promise.resolve --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index d23eb0f..d7ba0d8 100644 --- a/src/index.js +++ b/src/index.js @@ -60,7 +60,7 @@ const updateSession = (db, response) => { const session = parseCookie(response); if (session) { const sessionKey = getSessionKey(db); - sessions[sessionKey] = Promise.resolve(session); + sessions[sessionKey] = session; return session; } }; From bb239bef2ec216403d01fbc346d4dd200ed5fe84 Mon Sep 17 00:00:00 2001 From: Diana Barsan Date: Tue, 13 Feb 2024 15:46:28 +0700 Subject: [PATCH 4/6] no need to promise.resolve --- src/index.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/index.js b/src/index.js index d7ba0d8..4f794a5 100644 --- a/src/index.js +++ b/src/index.js @@ -82,10 +82,6 @@ const getSession = async (db) => { return sessions[sessionKey]; }; -const isExpired = (session) => { - return Date.now() > session?.expires; -}; - const extractAuth = (opts) => { if (opts.auth) { opts.credentials = opts.auth; @@ -103,11 +99,12 @@ const extractAuth = (opts) => { }; const validateSession = (db, session) => { - if (!session) { + if (!session || !session.expires) { return; } - if (isExpired(session)) { + const isExpired = Date.now() > session.expires; + if (isExpired) { invalidateSession(db); return getSession(db); } From ea0ff0e22c29fd1c4d9edae88a23f45cfd2ea2ac Mon Sep 17 00:00:00 2001 From: Diana Barsan Date: Tue, 13 Feb 2024 15:49:30 +0700 Subject: [PATCH 5/6] bump version --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2d195b6..3ac9ce1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "pouchdb-session-authentication", - "version": "1.0.0", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "pouchdb-session-authentication", - "version": "1.0.0", + "version": "1.1.0", "license": "AGPL-3.0-only", "dependencies": { "pouchdb-fetch": "^8.0.1" diff --git a/package.json b/package.json index e0d8c87..b011a9f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pouchdb-session-authentication", - "version": "1.0.0", + "version": "1.1.0", "description": "Plugin that forces session authentication for PouchDb http adapter", "main": "src/index.js", "scripts": { From 3faa21918bfed1d7f77927293853e48a727c6cf2 Mon Sep 17 00:00:00 2001 From: Diana Barsan Date: Tue, 13 Feb 2024 17:49:34 +0700 Subject: [PATCH 6/6] code review feedback! --- src/index.js | 36 ++++++++++++++++-------------------- test/unit/index.spec.js | 29 +++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/index.js b/src/index.js index 4f794a5..7854056 100644 --- a/src/index.js +++ b/src/index.js @@ -70,18 +70,6 @@ const invalidateSession = db => { delete sessions[sessionKey]; }; -const getSession = async (db) => { - const sessionKey = getSessionKey(db); - const session = sessions[sessionKey]; - - if (session) { - return session; - } - - sessions[sessionKey] = authenticate(db); - return sessions[sessionKey]; -}; - const extractAuth = (opts) => { if (opts.auth) { opts.credentials = opts.auth; @@ -98,18 +86,26 @@ const extractAuth = (opts) => { }; }; -const validateSession = (db, session) => { +const isValid = (session) => { if (!session || !session.expires) { - return; + return false; } - const isExpired = Date.now() > session.expires; - if (isExpired) { - invalidateSession(db); - return getSession(db); + return !isExpired; +}; + +const getValidSession = async (db) => { + const sessionKey = getSessionKey(db); + + if (sessions[sessionKey]) { + const session = await sessions[sessionKey]; + if (isValid(session)) { + return session; + } } - return session; + sessions[sessionKey] = authenticate(db); + return sessions[sessionKey]; }; // eslint-disable-next-line func-style @@ -124,7 +120,7 @@ function wrapAdapter (PouchDB, HttpPouch) { db.originalFetch = db.fetch || PouchDB.fetch; db.fetch = async (url, opts = {}) => { - const session = await validateSession(db, await getSession(db)); + const session = await getValidSession(db); if (session) { opts.headers = opts.headers || new Headers(); diff --git a/test/unit/index.spec.js b/test/unit/index.spec.js index 057b6b6..20a6be4 100644 --- a/test/unit/index.spec.js +++ b/test/unit/index.spec.js @@ -304,9 +304,9 @@ describe('Pouchdb Session authentication plugin', () => { await Promise.all([ db.fetch('randomUrl1'), - db.fetch('randomUrl2'), - db.fetch('randomUrl3'), - db.fetch('randomUrl4'), + db.fetch('randomUrl1'), + db.fetch('randomUrl1'), + db.fetch('randomUrl1'), ]); expect(fetch.callCount).to.equal(5); @@ -326,15 +326,15 @@ describe('Pouchdb Session authentication plugin', () => { { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } ]); expect(fetch.args[2]).to.deep.equal([ - 'randomUrl2', + 'randomUrl1', { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } ]); expect(fetch.args[3]).to.deep.equal([ - 'randomUrl3', + 'randomUrl1', { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } ]); expect(fetch.args[4]).to.deep.equal([ - 'randomUrl4', + 'randomUrl1', { headers: new Headers({ 'Cookie': 'AuthSession=theonetruesession' }) } ]); }); @@ -526,6 +526,23 @@ describe('Pouchdb Session authentication plugin', () => { } ]); expect(fetch.args[1]).to.deep.equal([ 'randomUrl', {} ]); + + const response2 = await db.fetch('randomUrl'); + + expect(response2).to.deep.equal({ ok: false, status: 401, body: 'omg' }); + expect(fetch.callCount).to.equal(4); + expect(fetch.args[2]).to.deep.equal([ + 'http://localhost:5984/_session', + { + method: 'POST', + headers: new Headers({ + 'Content-Type': 'application/json', + 'Accept': 'application/json', + }), + body: JSON.stringify({ name: 'admin', password: 'pass' }), + } + ]); + expect(fetch.args[3]).to.deep.equal([ 'randomUrl', {} ]); }); it('should continue when session cookie is not returned', async () => {