From 3a78cf48c9ae1af9cdb3b50a7765396d24fa7a0e Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Thu, 16 Nov 2023 12:01:50 +0100 Subject: [PATCH] Fixed deleting session when requesting identity for invalid session (#19017) ref https://ghost.slack.com/archives/C02G9E68C/p1700129928489809 - When the GET /api/session endpoint is called, the session is deleted if it is invalid - We don't have a body parser for this GET endoint, and the request object was passed to the deleteSession handler. This caused a type error (cannot read properties of undefined) - We had dangling promise because deleteSession is async and wasn't awaited, causing random errors in tests - Added a test that would have caught this earlier --- ghost/core/test/e2e-frontend/members.test.js | 31 ++++++++++++++++++++ ghost/members-ssr/lib/members-ssr.js | 4 +-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/ghost/core/test/e2e-frontend/members.test.js b/ghost/core/test/e2e-frontend/members.test.js index 80bb03f994..ae86c95c70 100644 --- a/ghost/core/test/e2e-frontend/members.test.js +++ b/ghost/core/test/e2e-frontend/members.test.js @@ -27,6 +27,12 @@ async function createMember(data) { }); } +async function cycleTransientId(data) { + return await members.api.members.cycleTransientId({ + ...data + }); +} + describe('Front-end members behavior', function () { let request; @@ -100,6 +106,7 @@ describe('Front-end members behavior', function () { it('should return no content when removing member sessions', async function () { await request.del('/members/api/session') + .expect('set-cookie', /ghost-members-ssr=.*;.*?expires=Thu, 01 Jan 1970 00:00:00 GMT;.*?/) .expect(204); }); @@ -550,6 +557,30 @@ describe('Front-end members behavior', function () { member = await loginAsMember('member1@test.com'); }); + it('an invalid token causes a set-cookie logout when requesting the identity', async function () { + // Check logged in + await request.get('/members/api/member') + .expect(200); + + // Cycle the transient id manually + await cycleTransientId({id: member.id}); + + await member.refresh(); + const transientId = member.get('transient_id'); + + await request.get('/members/api/session') + .expect('set-cookie', /ghost-members-ssr=.*;.*?expires=Thu, 01 Jan 1970 00:00:00 GMT;.*?/) + .expect(204); + + // Check logged out + await request.get('/members/api/member') + .expect(204); + + // Check transient id has NOT changed + await member.refresh(); + assert.equal(member.get('transient_id'), transientId); + }); + it('by default only destroys current session', async function () { const transientId = member.get('transient_id'); diff --git a/ghost/members-ssr/lib/members-ssr.js b/ghost/members-ssr/lib/members-ssr.js index 8b72462057..fed91c8dbd 100644 --- a/ghost/members-ssr/lib/members-ssr.js +++ b/ghost/members-ssr/lib/members-ssr.js @@ -267,7 +267,7 @@ class MembersSSR { * @returns {Promise} */ async deleteSession(req, res) { - if (req.body.all) { + if (req.body && typeof req.body === 'object' && req.body.all) { // Update transient_id to invalidate all sessions const member = await this.getMemberDataFromSession(req, res); if (member) { @@ -303,7 +303,7 @@ class MembersSSR { const transientId = this._getSessionCookies(req, res); const token = await this._getMemberIdentityToken(transientId); if (!token) { - this.deleteSession(req, res); + await this.deleteSession(req, res); throw new BadRequestError({ message: 'Invalid session, could not get identity token' });