OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Validated filters for collections

Browse Source 
refs https://github.com/TryGhost/Arch/issues/47

This ensures that we only have collections which have a valid filter in terms of
  - Valid NQL string
  - Uses only properties which are valid to filter on
  - Only has an empty filter in the case of the "latest" collection
This commit is contained in:
Fabien "egg" O'Carroll 2023-08-17 09:46:41 +01:00 committed by Fabien 'egg' O'Carroll
parent c98bf80248
commit 62d5ca558d
5 changed files with 124 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
ghost/collections/src/Collection.ts
View File

@ -18,6 +18,51 @@ const messages = {
slugMustBeUnique: 'Slug must be unique'
};
function validateFilter(filter: string | null, type: 'manual' | 'automatic', isAllowedEmpty = false) {
const allowedProperties = ['featured', 'published_at', 'tag', 'tags']
if (type === 'manual') {
if (filter !== null) {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message),
context: tpl(messages.invalidFilterProvided.context)
});
}
return;
}
// type === 'automatic' now
if (filter === null) {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message),
context: tpl(messages.invalidFilterProvided.context)
});
}
if (filter.trim() === '' && !isAllowedEmpty) {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message),
context: tpl(messages.invalidFilterProvided.context)
});
}
try {
const parsedFilter = nql(filter);
const keysUsed: string[] = [];
nql.utils.mapQuery(parsedFilter.toJSON(), function (value: unknown, key: string) {
keysUsed.push(key);
});
if (keysUsed.some(key => !allowedProperties.includes(key))) {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message)
});
}
} catch (err) {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message)
});
}
}
export class Collection {
id: string;
title: string;
@ -45,25 +90,11 @@ export class Collection {
return this._filter;
}
set filter(value) {
// Cannot change the filter of these collections
if (this.slug === 'latest' || this.slug === 'featured') {
return;
}
if (this.type === 'manual') {
if (value !== null) {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message),
context: tpl(messages.invalidFilterProvided.context)
});
}
} else {
if (value === null || value === '') {
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message),
context: tpl(messages.invalidFilterProvided.context)
});
}
}
validateFilter(value, this.type);
this._filter = value;
}
featureImage: string | null;
createdAt: Date;
@ -195,13 +226,9 @@ export class Collection {
});
}
if (data.type === 'automatic' && (data.slug !== 'latest') && !data.filter) {
// @NOTE: add filter validation here
throw new ValidationError({
message: tpl(messages.invalidFilterProvided.message),
context: tpl(messages.invalidFilterProvided.context)
});
}
const type = data.type === 'automatic' ? 'automatic' : 'manual';
const filter = typeof data.filter === 'string' ? data.filter : null;
validateFilter(filter, type, data.slug === 'latest');
if (!data.title) {
throw new ValidationError({
@ -214,8 +241,8 @@ export class Collection {
title: data.title,
slug: data.slug,
description: data.description || null,
type: data.type || 'manual',
filter: data.filter || null,
type: type,
filter: filter,
featureImage: data.feature_image || null,
createdAt: Collection.validateDateField(data.created_at, 'created_at'),
updatedAt: Collection.validateDateField(data.updated_at, 'updated_at'),

63
ghost/collections/test/Collection.test.ts
View File

@ -189,6 +189,43 @@ describe('Collection', function () {
assert.equal(collection.slug, 'edited-slug');
});
it('Throws when the collection filter is malformed', async function () {
const collection = await Collection.create({
title: 'Testing edits',
type: 'automatic',
filter: 'featured:true'
});
assert.throws(() => {
collection.filter = 'my name is, my name is, my name is, wicka wicka slim shady';
}, (err: any) => {
assert.equal(err.message, 'Invalid filter provided for automatic Collection', 'Error message should match');
return true;
});
});
it('Throws when the collection filter is invalid', async function () {
assert.rejects(async () => {
await Collection.create({
title: 'Testing creating collections with invalid filter',
type: 'automatic',
filter: 'unknown:egg'
});
});
const collection = await Collection.create({
title: 'Testing edits',
type: 'automatic',
filter: 'featured:true'
});
assert.throws(() => {
collection.filter = 'unknown:true';
}, (err: any) => {
assert.equal(err.message, 'Invalid filter provided for automatic Collection', 'Error message should match');
return true;
});
});
it('Throws when the collection filter is empty', async function () {
const collection = await Collection.create({
title: 'Testing edits',
@ -216,6 +253,32 @@ describe('Collection', function () {
collection.filter = '';
});
it('throws when trying to set an empty filter on an automatic collection', async function () {
assert.rejects(async () => {
await Collection.create({
title: 'Testing Creating Automatic With Empty Filter',
slug: 'testing-creating-automatic-with-empty-filter',
type: 'automatic',
filter: ''
});
});
const collection = await Collection.create({
title: 'Testing Editing Automatic With Empty Filter',
slug: 'testing-editing-automatic-with-empty-filter',
type: 'automatic',
filter: 'featured:true'
});
assert.throws(() => {
collection.filter = '';
}, (err: any) => {
assert.equal(err.message, 'Invalid filter provided for automatic Collection', 'Error message should match');
assert.equal(err.context, 'Automatic type of collection should always have a filter value', 'Error message should match');
return true;
});
});
it('throws when trying to set filter on a manual collection', async function () {
const collection = await Collection.create({
title: 'Testing Manual Filter',

2
ghost/collections/test/collections.test.ts
View File

@ -276,7 +276,7 @@ describe('CollectionsService', function () {
let updatedCollection = await collectionsService.edit({
id: collection.id,
filter: 'id:post-3-featured'
filter: 'featured:true+published_at:>2023-05-20'
});
assert.equal(updatedCollection?.posts.length, 1, 'Collection should have one post');

8
ghost/core/test/e2e-api/admin/__snapshots__/collections.test.js.snap
View File

@ -1262,7 +1262,7 @@ Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"description": "All posts",
"feature_image": null,
"filter": null,
"filter": "",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"slug": "latest",
"title": "Latest",
@ -1298,7 +1298,7 @@ exports[`Collections API Browse Can browse Collections 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "578",
"content-length": "576",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
@ -1317,7 +1317,7 @@ Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"description": "All posts",
"feature_image": null,
"filter": null,
"filter": "",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"slug": "latest",
"title": "Latest",
@ -1356,7 +1356,7 @@ exports[`Collections API Browse Can browse Collections and include the posts cou
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "619",
"content-length": "617",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,

8
ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap
View File

@ -1587,7 +1587,7 @@ Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.\\\\d\\{3\\}Z/,
"description": "All posts",
"feature_image": null,
"filter": null,
"filter": "",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"posts": Array [
Object {
@ -1758,7 +1758,7 @@ exports[`Posts API Update Can add and remove collections 4: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "5209",
"content-length": "5207",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
@ -1798,7 +1798,7 @@ Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.\\\\d\\{3\\}Z/,
"description": "All posts",
"feature_image": null,
"filter": null,
"filter": "",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"posts": Array [
Object {
@ -1969,7 +1969,7 @@ exports[`Posts API Update Can add and remove collections 6: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "5203",
"content-length": "5201",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,