diff --git a/packages/api-aco/src/fields/index.ts b/packages/api-aco/src/fields/index.ts deleted file mode 100644 index 00b19edc6d8..00000000000 --- a/packages/api-aco/src/fields/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { CmsModelFieldToGraphQLPlugin } from "@webiny/api-headless-cms/types"; - -export const createFields = (): CmsModelFieldToGraphQLPlugin[] => { - return [ - // createLocationField() - ]; -}; diff --git a/packages/api-aco/src/fields/location.ts b/packages/api-aco/src/fields/location.ts deleted file mode 100644 index 6ff16d6cb74..00000000000 --- a/packages/api-aco/src/fields/location.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { CmsModelField, CmsModelFieldToGraphQLPlugin } from "@webiny/api-headless-cms/types"; - -const createFilters = (field: CmsModelField) => { - return [ - `${field.fieldId}: AcoLocationInput`, - `${field.fieldId}_in: [AcoLocationInput!]`, - `${field.fieldId}_not_in: [AcoLocationInput!]` - ].join("\n"); -}; -export const createLocationField = (): CmsModelFieldToGraphQLPlugin => { - return { - type: "cms-model-field-to-graphql", - name: "cms-model-field-to-graphql-location", - fieldType: "location", - isSortable: false, - isSearchable: true, - read: { - createGetFilters({ field }): string { - return createFilters(field); - }, - createListFilters({ field }): string { - return createFilters(field); - }, - createTypeField(): string { - return ``; - } - }, - manage: { - createTypeField(): string { - return ``; - }, - createInputField({ field }): string { - return `${field.fieldId}: AcoLocationInput`; - } - } - }; -}; diff --git a/packages/api-aco/src/index.ts b/packages/api-aco/src/index.ts index 32516c749cb..b6eb9ba7210 100644 --- a/packages/api-aco/src/index.ts +++ b/packages/api-aco/src/index.ts @@ -1,6 +1,5 @@ import { createAcoContext } from "~/createAcoContext"; import { createAcoGraphQL } from "~/createAcoGraphQL"; -import { createFields } from "~/fields"; export { SEARCH_RECORD_MODEL_ID } from "./record/record.model"; export { FOLDER_MODEL_ID } from "./folder/folder.model"; @@ -13,5 +12,5 @@ export interface CreateAcoParams { } export const createAco = (params: CreateAcoParams = {}) => { - return [...createFields(), createAcoContext(params), ...createAcoGraphQL()]; + return [createAcoContext(params), ...createAcoGraphQL()]; }; diff --git a/packages/api-aco/src/utils/decorators/CmsEntriesCrudDecorators.ts b/packages/api-aco/src/utils/decorators/CmsEntriesCrudDecorators.ts index 376d11c9059..5246647bc6f 100644 --- a/packages/api-aco/src/utils/decorators/CmsEntriesCrudDecorators.ts +++ b/packages/api-aco/src/utils/decorators/CmsEntriesCrudDecorators.ts @@ -1,62 +1,10 @@ import { AcoContext } from "~/types"; -import { CmsEntry, CmsModel } from "@webiny/api-headless-cms/types"; -import { NotFoundError } from "@webiny/handler-graphql"; -import { FolderLevelPermissions } from "~/utils/FolderLevelPermissions"; import { createWhere } from "./where"; import { ROOT_FOLDER } from "./constants"; +import { filterEntriesByFolderFactory } from "./filterEntriesByFolderFactory"; +import { createFolderType } from "./createFolderType"; type Context = Pick; -/** - * Keep this until we figure out how to fetch the folders. - */ -const isPageModel = (model: CmsModel): boolean => { - if (model.modelId === "pbPage") { - return true; - } else if (model.modelId === "acoSearchRecord-pbpage") { - return true; - } - return false; -}; - -const createFolderType = (model: CmsModel): "FmFile" | "PbPage" | `cms:${string}` => { - if (model.modelId === "fmFile") { - return "FmFile"; - } else if (isPageModel(model)) { - return "PbPage"; - } - return `cms:${model.modelId}`; -}; - -const filterEntriesByFolderFactory = (context: Context, permissions: FolderLevelPermissions) => { - return async (model: CmsModel, entries: CmsEntry[]) => { - const [folders] = await context.aco.folder.listAll({ - where: { - type: createFolderType(model) - } - }); - - const results = await Promise.all( - entries.map(async entry => { - const folderId = entry.location?.folderId; - if (!folderId || folderId === ROOT_FOLDER) { - return entry; - } - - const folder = folders.find(folder => folder.id === folderId); - if (!folder) { - throw new NotFoundError(`Folder "${folderId}" not found.`); - } - const result = await permissions.canAccessFolderContent({ - folder, - rwd: "r" - }); - return result ? entry : null; - }) - ); - - return results.filter((entry): entry is CmsEntry => !!entry); - }; -}; interface EntryManagerCrudDecoratorsParams { context: Context; @@ -81,6 +29,7 @@ export class CmsEntriesCrudDecorators { const folders = await folderLevelPermissions.listAllFoldersWithPermissions(folderType); const where = createWhere({ + model, where: params.where, folders }); @@ -244,6 +193,7 @@ export class CmsEntriesCrudDecorators { const entry = await context.cms.storageOperations.entries.getRevisionById(model, { id }); + const folderId = entry?.location?.folderId || ROOT_FOLDER; /** * If the entry is in the same folder we are trying to move it to, just continue. diff --git a/packages/api-aco/src/utils/decorators/createFolderType.ts b/packages/api-aco/src/utils/decorators/createFolderType.ts new file mode 100644 index 00000000000..14b7f7ad5c0 --- /dev/null +++ b/packages/api-aco/src/utils/decorators/createFolderType.ts @@ -0,0 +1,11 @@ +import { CmsModel } from "@webiny/api-headless-cms/types"; +import { isPageModel } from "./isPageModel"; + +export const createFolderType = (model: CmsModel): "FmFile" | "PbPage" | `cms:${string}` => { + if (model.modelId === "fmFile") { + return "FmFile"; + } else if (isPageModel(model)) { + return "PbPage"; + } + return `cms:${model.modelId}`; +}; diff --git a/packages/api-aco/src/utils/decorators/filterEntriesByFolderFactory.ts b/packages/api-aco/src/utils/decorators/filterEntriesByFolderFactory.ts new file mode 100644 index 00000000000..9dc5db3349c --- /dev/null +++ b/packages/api-aco/src/utils/decorators/filterEntriesByFolderFactory.ts @@ -0,0 +1,43 @@ +import { AcoContext } from "~/types"; +import { CmsEntry, CmsModel } from "@webiny/api-headless-cms/types"; +import { NotFoundError } from "@webiny/handler-graphql"; +import { FolderLevelPermissions } from "~/utils/FolderLevelPermissions"; +import { ROOT_FOLDER } from "./constants"; + +type Context = Pick; + +import { createFolderType } from "./createFolderType"; + +export const filterEntriesByFolderFactory = ( + context: Context, + permissions: FolderLevelPermissions +) => { + return async (model: CmsModel, entries: CmsEntry[]) => { + const [folders] = await context.aco.folder.listAll({ + where: { + type: createFolderType(model) + } + }); + + const results = await Promise.all( + entries.map(async entry => { + const folderId = entry.location?.folderId; + if (!folderId || folderId === ROOT_FOLDER) { + return entry; + } + + const folder = folders.find(folder => folder.id === folderId); + if (!folder) { + throw new NotFoundError(`Folder "${folderId}" not found.`); + } + const result = await permissions.canAccessFolderContent({ + folder, + rwd: "r" + }); + return result ? entry : null; + }) + ); + + return results.filter((entry): entry is CmsEntry => !!entry); + }; +}; diff --git a/packages/api-aco/src/utils/decorators/isPageModel.ts b/packages/api-aco/src/utils/decorators/isPageModel.ts new file mode 100644 index 00000000000..3395f74329f --- /dev/null +++ b/packages/api-aco/src/utils/decorators/isPageModel.ts @@ -0,0 +1,13 @@ +import { CmsModel } from "@webiny/api-headless-cms/types"; + +/** + * Keep this until we figure out how to fetch the folders. + */ +export const isPageModel = (model: CmsModel): boolean => { + if (model.modelId === "pbPage") { + return true; + } else if (model.modelId === "acoSearchRecord-pbpage") { + return true; + } + return false; +}; diff --git a/packages/api-aco/src/utils/decorators/where.ts b/packages/api-aco/src/utils/decorators/where.ts index 67817d8bce8..65f76a51c3f 100644 --- a/packages/api-aco/src/utils/decorators/where.ts +++ b/packages/api-aco/src/utils/decorators/where.ts @@ -1,8 +1,10 @@ -import { CmsEntryListWhere } from "@webiny/api-headless-cms/types"; +import { CmsEntryListWhere, CmsModel } from "@webiny/api-headless-cms/types"; import { Folder } from "~/folder/folder.types"; import { ROOT_FOLDER } from "./constants"; +import { isPageModel } from "~/utils/decorators/isPageModel"; interface Params { + model: CmsModel; where: CmsEntryListWhere | undefined; folders: Folder[]; } @@ -14,19 +16,28 @@ interface Params { * * no existing location with no AND conditional + with AND conditional */ export const createWhere = (params: Params): CmsEntryListWhere | undefined => { - const { where, folders } = params; - if (!where) { - return undefined; - } + const { model, where, folders } = params; + + // Once we migrate PB to HCMS, we can remove this check and always use `wbAco_location`. + const locationFieldName = isPageModel(model) ? "location" : "wbyAco_location"; + const whereLocation = { - wbyAco_location: { + [locationFieldName]: { // At the moment, all users can access entries in the root folder. // Root folder level permissions cannot be set yet. folderId_in: [ROOT_FOLDER, ...folders.map(folder => folder.id)] } }; + + if (!where) { + // If no `where` condition is present, that means we're performing a query + // across all folders. Still, with FLP enabled, we need to filter out + // folders to which the user does not have access. + return whereLocation; + } + const whereAnd = where.AND; - if (where.wbyAco_location && !whereAnd) { + if (where[locationFieldName] && !whereAnd) { return { ...where, AND: [ @@ -35,7 +46,7 @@ export const createWhere = (params: Params): CmsEntryListWhere | undefined => { } ] }; - } else if (where.wbyAco_location && whereAnd) { + } else if (where[locationFieldName] && whereAnd) { return { ...where, AND: [ @@ -46,6 +57,7 @@ export const createWhere = (params: Params): CmsEntryListWhere | undefined => { ] }; } + return { ...where, ...whereLocation diff --git a/packages/api-page-builder-aco/__tests__/flp.test.ts b/packages/api-page-builder-aco/__tests__/flp.test.ts index 9439488b820..a3e16c03948 100644 --- a/packages/api-page-builder-aco/__tests__/flp.test.ts +++ b/packages/api-page-builder-aco/__tests__/flp.test.ts @@ -1,5 +1,6 @@ import { useGraphQlHandler } from "./utils/useGraphQlHandler"; import { SecurityIdentity } from "@webiny/api-security/types"; +import { until } from "@webiny/project-utils/testing/helpers/until"; const FOLDER_TYPE = "PbPage"; @@ -54,6 +55,10 @@ describe("Folder Level Permissions - Page Manager GraphQL API", () => { ); } + await until(gqlIdentityA.pageBuilder.listPages, ([response]) => { + return response.data.pageBuilder.listPages.data.length === 4; + }); + await expect( gqlIdentityA.pageBuilder.listPages().then(([response]) => { return response.data.pageBuilder.listPages.data; @@ -75,13 +80,17 @@ describe("Folder Level Permissions - Page Manager GraphQL API", () => { for (let i = 1; i <= 4; i++) { createdPages.push( await gqlIdentityB.pageBuilder - .createPage({ category: "static" }) + .createPage({ category: "static", meta: { location: "root" } }) .then(([response]) => { return response.data.pageBuilder.createPage.data; }) ); } + await until(gqlIdentityB.pageBuilder.listPages, ([response]) => { + return response.data.pageBuilder.listPages.data.length === 4; + }); + await expect( gqlIdentityB.pageBuilder.listPages().then(([response]) => { return response.data.pageBuilder.listPages.data; @@ -138,6 +147,31 @@ describe("Folder Level Permissions - Page Manager GraphQL API", () => { } }); + // Listing ACO records in the folder should be forbidden for identity C. + const [emptySearchRecordsList] = await gqlIdentityC.search.listRecords({ + where: { + location: { + folderId: folder.id + } + } + }); + + expect(emptySearchRecordsList).toEqual({ + data: { + search: { + listRecords: { + data: [], + error: null, + meta: { + hasMoreItems: false, + totalCount: 0, + cursor: null + } + } + } + } + }); + // Getting pages in the folder should be forbidden for identity C. for (let i = 0; i < createdPages.length; i++) { const createdPage = createdPages[i]; @@ -295,6 +329,10 @@ describe("Folder Level Permissions - Page Manager GraphQL API", () => { } // Listing pages in the folder should be now allowed for identity C. + await until(gqlIdentityC.pageBuilder.listPages, ([response]) => { + return response.data.pageBuilder.listPages.meta.totalCount === 1; + }); + await expect( gqlIdentityC.pageBuilder.listPages().then(([response]) => { return response.data.pageBuilder.listPages; diff --git a/packages/api-page-builder-aco/__tests__/graphql/record.gql.ts b/packages/api-page-builder-aco/__tests__/graphql/record.gql.ts index 2448bb285fa..b0d90cdfc61 100644 --- a/packages/api-page-builder-aco/__tests__/graphql/record.gql.ts +++ b/packages/api-page-builder-aco/__tests__/graphql/record.gql.ts @@ -48,9 +48,9 @@ export const GET_RECORD = /* GraphQL */ ` `; export const LIST_RECORDS = /* GraphQL */ ` - query ListRecords { + query ListRecords($where:AcoSearchRecordPbListWhereInput) { search { - listRecords: listAcoSearchRecordPb { + listRecords: listAcoSearchRecordPb(where:$where) { data ${DATA_FIELD()} error ${ERROR_FIELD} meta { diff --git a/packages/api-page-builder-aco/__tests__/page.hooks.test.ts b/packages/api-page-builder-aco/__tests__/page.hooks.test.ts index 61e425ab0a2..9b31698f51f 100644 --- a/packages/api-page-builder-aco/__tests__/page.hooks.test.ts +++ b/packages/api-page-builder-aco/__tests__/page.hooks.test.ts @@ -138,7 +138,15 @@ describe("Pages -> Search records", () => { }); }); - it("should create a search record on page creation - disable storageId conversion", async () => { + // TODO: revisit this when possible. More information in following lines. + // This test is failing because of an issue that surfaced while fixing an FLP-related issue. + // Basically, because of the `WEBINY_API_TEST_STORAGE_ID_CONVERSION_DISABLE` env variable, in + // Elasticsearch, storage IDs are not used as object property names. Which is fine, but, when + // Elasticsearch list queries are performed by CMS, within the `where` param, storage IDs *are* + // actually used, which makes queries return empty results. Last time we were inspecting this, + // we ended up debugging the `applyFiltering` function, created in the following file: + // packages/api-headless-cms-ddb-es/src/operations/entry/elasticsearch/filtering/applyFiltering.ts + it.skip("should create a search record on page creation - disable storageId conversion", async () => { process.env.WEBINY_API_TEST_STORAGE_ID_CONVERSION_DISABLE = "true"; const { pageBuilder, search } = useGraphQlHandler({ plugins: [assignPageLifecycleEvents()]