mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-12-24 03:14:03 +03:00
Removed members caching cookies when no member is logged in (#20349)
ref https://linear.app/tryghost/issue/KTLO-58/dont-send-ghost-acess-cookies-if-no-member-is-logged-in - Currently when member's caching is enabled, but no member is logged in, we always send `ghost-access=null;` and `ghost-access-hmac=null;` cookies in the requests to `/members/api/member/`. This is done to clear the cookies, but an unintended consequence is that these requests can never be cached since there is a cookie in the response. - This PR removes the cookies from the requests when no member is logged in, the cookies will not be sent, allowing the requests to be cached - It also unsets the cookies when deleting a member's session, so that the cookies are not sent in the requests after the member logs out - This should improve the cache hit ratio with members caching enabled
This commit is contained in:
Chris Raible
GitHub
parent
58dcbe9096
commit
4a6d427673
@ -29,12 +29,19 @@ const getFreeTier = async function getFreeTier() {
|
||||
|
||||
/**
|
||||
* Sets the ghost-access and ghost-access-hmac cookies on the response object
|
||||
* @param {object} member - The member object
|
||||
* @param {Object} member - The member object
|
||||
* @param {import('express').Request} req - The member object
|
||||
* @param {import('express').Response} res - The express response object to set the cookies on
|
||||
* @param {Object} freeTier - The free tier object
|
||||
* @returns
|
||||
*/
|
||||
const setAccessCookies = function setAccessCookies(member = undefined, res, freeTier) {
|
||||
const setAccessCookies = function setAccessCookies(member, req, res, freeTier) {
|
||||
if (!member) {
|
||||
// If there is no cookie sent with the request, return early
|
||||
if (!req.headers.cookie || !req.headers.cookie.includes('ghost-access')) {
|
||||
return;
|
||||
}
|
||||
// If there are cookies sent with the request, set them to null and expire them immediately
|
||||
const accessCookie = `ghost-access=null; Max-Age=0; Path=/; HttpOnly; SameSite=Strict;`;
|
||||
const hmacCookie = `ghost-access-hmac=null; Max-Age=0; Path=/; HttpOnly; SameSite=Strict;`;
|
||||
const existingCookies = res.getHeader('Set-Cookie') || [];
|
||||
@ -70,7 +77,7 @@ const setAccessCookies = function setAccessCookies(member = undefined, res, free
|
||||
const accessInfoSession = async function accessInfoSession(req, res, next) {
|
||||
const freeTier = await getFreeTier();
|
||||
onHeaders(res, function () {
|
||||
setAccessCookies(req.member, res, freeTier);
|
||||
setAccessCookies(req.member, req, res, freeTier);
|
||||
});
|
||||
next();
|
||||
};
|
||||
@ -304,7 +311,7 @@ const createSessionFromMagicLink = async function createSessionFromMagicLink(req
|
||||
// Set the ghost-access cookies to enable tier-based caching
|
||||
try {
|
||||
const freeTier = await getFreeTier();
|
||||
setAccessCookies(member, res, freeTier);
|
||||
setAccessCookies(member, req, res, freeTier);
|
||||
} catch {
|
||||
// This is a non-critical operation, so we can safely ignore any errors
|
||||
}
|
||||
|
||||
@ -461,6 +461,58 @@ Object {
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Comments API when caching members content is enabled sets ghost-access and ghost-access-hmac cookies to null when not authenticated but a cookie is sent 1: [body] 1`] = `
|
||||
Object {
|
||||
"avatar_image": null,
|
||||
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
|
||||
"email": "member@example.com",
|
||||
"email_suppression": Object {
|
||||
"info": null,
|
||||
"suppressed": false,
|
||||
},
|
||||
"enable_comment_notifications": true,
|
||||
"expertise": null,
|
||||
"firstname": null,
|
||||
"name": null,
|
||||
"newsletters": Array [
|
||||
Object {
|
||||
"description": null,
|
||||
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
|
||||
"name": "Default Newsletter",
|
||||
"sort_order": 0,
|
||||
"uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
|
||||
},
|
||||
Object {
|
||||
"description": null,
|
||||
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
|
||||
"name": "Weekly newsletter",
|
||||
"sort_order": 2,
|
||||
"uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
|
||||
},
|
||||
],
|
||||
"paid": false,
|
||||
"subscribed": false,
|
||||
"subscriptions": Array [],
|
||||
"uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Comments API when caching members content is enabled sets ghost-access and ghost-access-hmac cookies to null when not authenticated but a cookie is sent 2: [headers] 1`] = `
|
||||
Object {
|
||||
"access-control-allow-origin": "*",
|
||||
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
|
||||
"content-length": "621",
|
||||
"content-type": "application/json; charset=utf-8",
|
||||
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
|
||||
"set-cookie": Array [
|
||||
StringMatching /\\^ghost-access=\\[0-9a-fA-F\\]\\{24\\}:\\\\d\\{10\\}/,
|
||||
StringMatching /\\^ghost-access-hmac=\\[a-fA-F0-9\\]\\{64\\}/,
|
||||
],
|
||||
"vary": "Accept-Encoding",
|
||||
"x-powered-by": "Express",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Comments API when not authenticated but enabled can update comment notifications 1: [body] 1`] = `
|
||||
Object {
|
||||
"email": "member1@test.com",
|
||||
|
||||
@ -249,5 +249,37 @@ describe('Comments API', function () {
|
||||
body.email.should.eql(member.get('email'));
|
||||
});
|
||||
});
|
||||
|
||||
it('does not set ghost-access and ghost-access-hmac cookies when not authenticated', async function () {
|
||||
configUtils.set('cacheMembersContent:enabled', true);
|
||||
configUtils.set('cacheMembersContent:hmacSecret', crypto.randomBytes(64).toString('base64'));
|
||||
membersAgent = await agentProvider.getMembersAPIAgent();
|
||||
await fixtureManager.init('newsletters', 'members:newsletters');
|
||||
await membersAgent
|
||||
.get(`/api/member/`)
|
||||
.expectStatus(204)
|
||||
.expectEmptyBody()
|
||||
.expect(({headers}) => {
|
||||
should.not.exist(headers['set-cookie']);
|
||||
});
|
||||
});
|
||||
|
||||
it('sets ghost-access and ghost-access-hmac cookies to null when not authenticated but a cookie is sent', async function () {
|
||||
// This is to ensure that the cookies are reset when a user logs out
|
||||
configUtils.set('cacheMembersContent:enabled', true);
|
||||
configUtils.set('cacheMembersContent:hmacSecret', crypto.randomBytes(64).toString('base64'));
|
||||
membersAgent = await agentProvider.getMembersAPIAgent();
|
||||
await fixtureManager.init('newsletters', 'members:newsletters');
|
||||
// Send a ghost-access cookie but without a valid member session
|
||||
await membersAgent.jar.setCookie('ghost-access=fake;');
|
||||
await membersAgent
|
||||
.get('/api/member/')
|
||||
.expect(({headers}) => {
|
||||
should.exist(headers['set-cookie']);
|
||||
headers['set-cookie'].should.matchAny(/ghost-access=null;/);
|
||||
})
|
||||
.expectStatus(204)
|
||||
.expectEmptyBody();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -111,6 +111,11 @@ class MembersSSR {
|
||||
_removeSessionCookie(req, res) {
|
||||
const cookies = this._getCookies(req, res);
|
||||
cookies.set(this.sessionCookieName, null, this.sessionCookieOptions);
|
||||
// If members caching cookies are set, remove them
|
||||
if (cookies.get('ghost-access') || cookies.get('ghost-access-hmac')) {
|
||||
cookies.set('ghost-access', null, {...this.sessionCookieOptions, signed: false});
|
||||
cookies.set('ghost-access-hmac', null, {...this.sessionCookieOptions, signed: false});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Loading…
Reference in New Issue
Block a user