From 630abd8a2476121fa319e65daae4e168b0c7cf34 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 16 Feb 2024 14:13:04 +0100 Subject: [PATCH] Do not allow duplicated compliance topics --- .../app/src/cli/models/app/loader.test.ts | 110 +++--------------- .../validation/app_config_webhook.ts | 27 ++--- .../src/cli/services/app/config/link.test.ts | 9 +- packages/app/src/cli/services/context.ts | 2 +- packages/app/src/cli/services/info.test.ts | 2 +- .../partners-client.ts | 6 +- .../cli-kit/src/public/common/array.test.ts | 15 ++- packages/cli-kit/src/public/common/array.ts | 10 ++ 8 files changed, 64 insertions(+), 117 deletions(-) diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index 24e2c7601d..5bd39b4799 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -2681,145 +2681,67 @@ describe('WebhooksSchema', () => { expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig) }) - test('does not allow identical compliance_topics and uri', async () => { + test('throws an error if we have privacy_compliance section and subscriptions with compliance_topics', async () => { const webhookConfig: WebhooksConfig = { api_version: '2021-07', + privacy_compliance: { + customer_data_request_url: 'https://example.com', + }, subscriptions: [ { - topics: ['metaobjects/create'], - uri: 'https://example.com', - sub_topic: 'type:metaobject_one', - compliance_topics: ['shop/redact'], - }, - { - topics: ['metaobjects/create'], + compliance_topics: ['customers/data_request'], uri: 'https://example.com', - sub_topic: 'type:metaobject_two', - compliance_topics: ['shop/redact'], }, ], } const errorObj = { code: zod.ZodIssueCode.custom, - message: 'You can’t have duplicate privacy compliance subscriptions with the exact same `uri`', - fatal: true, - path: ['webhooks', 'subscriptions', 1, 'compliance_topics', 0, 'shop/redact'], + message: `The privacy_compliance section can't be used if there are subscriptions including compliance_topics`, + path: ['webhooks'], } const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig) expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp', [errorObj]) }) - test('does not allow identical compliance_topics in same subscription (will get by zod enum validation)', async () => { + test('throws an error if neither topics nor compliance_topics are added', async () => { const webhookConfig: WebhooksConfig = { api_version: '2021-07', subscriptions: [ { - topics: ['metaobjects/create'], uri: 'https://example.com', - sub_topic: 'type:metaobject_one', - compliance_topics: ['shop/redact', 'shop/redact'], }, ], } const errorObj = { code: zod.ZodIssueCode.custom, - message: 'You can’t have duplicate privacy compliance subscriptions with the exact same `uri`', - fatal: true, - path: ['webhooks', 'subscriptions', 0, 'compliance_topics', 1, 'shop/redact'], + message: 'Either topics or compliance_topics must be added to the webhook subscription', + path: ['webhooks', 'subscriptions', 0], } const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig) expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp', [errorObj]) }) - test('allows same compliance_topics if uri is different', async () => { + test('throws an error when there are duplicated compliance topics', async () => { const webhookConfig: WebhooksConfig = { api_version: '2021-07', subscriptions: [ { - topics: ['metaobjects/create'], uri: 'https://example.com', - sub_topic: 'type:metaobject_one', - compliance_topics: ['shop/redact'], - }, - { - topics: ['products/create'], - uri: 'https://example-two.com', - sub_topic: 'type:metaobject_two', - compliance_topics: ['shop/redact'], - }, - ], - } - - const {abortOrReport, parsedConfiguration} = await setupParsing({}, webhookConfig) - expect(abortOrReport).not.toHaveBeenCalled() - expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig) - }) - - test('allows same compliance_topics across https, pub sub and arn with multiple topics', async () => { - const webhookConfig: WebhooksConfig = { - api_version: '2021-07', - subscriptions: [ - { - topics: ['products/create'], - uri: 'https://example.com/all_webhooks', - compliance_topics: ['shop/redact', 'customers/data_request', 'customers/redact'], - }, - { - topics: ['products/create'], - uri: 'pubsub://my-project-123:my-topic', - compliance_topics: ['customers/data_request', 'customers/redact'], - }, - { - topics: ['products/create'], - uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/123/compliance', - compliance_topics: ['shop/redact', 'customers/redact'], - }, - ], - } - - const {abortOrReport, parsedConfiguration} = await setupParsing({}, webhookConfig) - expect(abortOrReport).not.toHaveBeenCalled() - expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig) - }) - - test('throws an error if we have privacy_compliance section and subscriptions with compliance_topics', async () => { - const webhookConfig: WebhooksConfig = { - api_version: '2021-07', - privacy_compliance: { - customer_data_request_url: 'https://example.com', - }, - subscriptions: [ - { compliance_topics: ['customers/data_request'], - uri: 'https://example.com', }, - ], - } - const errorObj = { - code: zod.ZodIssueCode.custom, - message: `The privacy_compliance section can't be used if there are subscriptions including compliance_topics`, - path: ['webhooks'], - } - - const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig) - expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp', [errorObj]) - }) - - test('throws an error if neither topics nor compliance_topics are added', async () => { - const webhookConfig: WebhooksConfig = { - api_version: '2021-07', - subscriptions: [ { - uri: 'https://example.com', + uri: 'https://example.com/other', + compliance_topics: ['customers/data_request'], }, ], } const errorObj = { code: zod.ZodIssueCode.custom, - message: 'Either topics or compliance_topics must be added to the webhook subscription', - path: ['webhooks', 'subscriptions', 0], + message: 'You can’t have duplicate subscriptions with the same compliance topic', + fatal: true, + path: ['webhooks', 'subscriptions'], } const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig) diff --git a/packages/app/src/cli/models/extensions/specifications/validation/app_config_webhook.ts b/packages/app/src/cli/models/extensions/specifications/validation/app_config_webhook.ts index 0d80c97176..ab5cc71c56 100644 --- a/packages/app/src/cli/models/extensions/specifications/validation/app_config_webhook.ts +++ b/packages/app/src/cli/models/extensions/specifications/validation/app_config_webhook.ts @@ -1,4 +1,5 @@ import {zod} from '@shopify/cli-kit/node/schema' +import {uniq} from '@shopify/cli-kit/common/array' import type {WebhooksConfig} from '../types/app_config_webhook.js' export function webhookValidator(schema: object, ctx: zod.RefinementCtx) { @@ -13,7 +14,6 @@ export function webhookValidator(schema: object, ctx: zod.RefinementCtx) { function validateSubscriptions(webhookConfig: WebhooksConfig) { const {subscriptions = []} = webhookConfig const uniqueSubscriptionSet = new Set() - const uniqueComplianceSubscriptionSet = new Set() if (!subscriptions.length) return @@ -27,6 +27,16 @@ function validateSubscriptions(webhookConfig: WebhooksConfig) { } } + const ComplianceTopics = subscriptions.flatMap((subscription) => subscription.compliance_topics).filter(Boolean) + if (uniq(ComplianceTopics).length !== ComplianceTopics.length) { + return { + code: zod.ZodIssueCode.custom, + message: 'You can’t have duplicate subscriptions with the same compliance topic', + fatal: true, + path: ['subscriptions'], + } + } + // eslint-disable-next-line @typescript-eslint/naming-convention for (const [i, {uri, topics = [], compliance_topics = [], sub_topic = ''}] of subscriptions.entries()) { const path = ['subscriptions', i] @@ -53,20 +63,5 @@ function validateSubscriptions(webhookConfig: WebhooksConfig) { uniqueSubscriptionSet.add(key) } - - for (const [j, complianceTopic] of compliance_topics.entries()) { - const key = `${complianceTopic}::${uri}` - - if (uniqueComplianceSubscriptionSet.has(key)) { - return { - code: zod.ZodIssueCode.custom, - message: 'You can’t have duplicate privacy compliance subscriptions with the exact same `uri`', - fatal: true, - path: [...path, 'compliance_topics', j, complianceTopic], - } - } - - uniqueComplianceSubscriptionSet.add(key) - } } } diff --git a/packages/app/src/cli/services/app/config/link.test.ts b/packages/app/src/cli/services/app/config/link.test.ts index 899ffd32ad..332bbef4d3 100644 --- a/packages/app/src/cli/services/app/config/link.test.ts +++ b/packages/app/src/cli/services/app/config/link.test.ts @@ -14,6 +14,7 @@ import {AppInterface, CurrentAppConfiguration} from '../../../models/app/app.js' import {fetchAppRemoteConfiguration} from '../select-app.js' import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' import {OrganizationApp} from '../../../models/organization.js' +import {fetchAppExtensionRegistrations} from '../../dev/fetch.js' import {beforeEach, describe, expect, test, vi} from 'vitest' import {fileExistsSync, inTemporaryDirectory, readFile, writeFileSync} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' @@ -565,8 +566,9 @@ embedded = false // Given const options: LinkOptions = { directory: tmp, - commandConfig: {runHook: vi.fn(() => Promise.resolve({successes: []}))} as unknown as Config, + // commandConfig: {runHook: vi.fn(() => Promise.resolve({successes: []}))} as unknown as Config, } + vi.mocked(loadApp).mockRejectedValue('App not found') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp()) vi.mocked(fetchAppExtensionRegistrations).mockResolvedValue({ @@ -590,6 +592,11 @@ embedded = false dashboardManagedExtensionRegistrations: [], }, }) + const remoteConfiguration = { + ...DEFAULT_REMOTE_CONFIGURATION, + access_scopes: {scopes: 'read_products,write_orders'}, + } + vi.mocked(fetchAppRemoteConfiguration).mockResolvedValue(remoteConfiguration) // When await link(options) diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index 20f1beb774..5b34ea984d 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -202,7 +202,7 @@ export async function ensureDevContext( selectedApp.apiKey, developerPlatformClient, specifications, - selectedApp.betas, + selectedApp.flags, ), } diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index a20736e761..5cf0f3fb55 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -30,7 +30,7 @@ const APP1 = testOrganizationApp({id: '123', title: 'my app', apiKey: '12345'}) const ORG1 = { id: '123', - betas: {}, + flags: {}, businessName: 'test', website: '', apps: {nodes: []}, diff --git a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts index 757571f5bb..5aac178e02 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts @@ -10,7 +10,7 @@ import { fetchOrganizations, fetchOrgAndApps, fetchOrgFromId, - filterDisabledBetas, + filterDisabledFlags, } from '../../../cli/services/dev/fetch.js' import {MinimalOrganizationApp, Organization, OrganizationApp, OrganizationStore} from '../../models/organization.js' import {selectOrganizationPrompt} from '../../prompts/dev.js' @@ -170,8 +170,8 @@ export class PartnersClient implements DeveloperPlatformClient { throw new AbortError(errors) } - const betas = filterDisabledBetas(result.appCreate.app.disabledBetas) - return {...result.appCreate.app, organizationId: org.id, newApp: true, betas} + const flags = filterDisabledFlags(result.appCreate.app.disabledFlags) + return {...result.appCreate.app, organizationId: org.id, newApp: true, flags} } async devStoresForOrg(orgId: string): Promise { diff --git a/packages/cli-kit/src/public/common/array.test.ts b/packages/cli-kit/src/public/common/array.test.ts index 2b2bf088f6..720becbac0 100644 --- a/packages/cli-kit/src/public/common/array.test.ts +++ b/packages/cli-kit/src/public/common/array.test.ts @@ -1,4 +1,4 @@ -import {difference, uniqBy} from './array.js' +import {difference, uniq, uniqBy} from './array.js' import {describe, test, expect} from 'vitest' describe('uniqBy', () => { @@ -36,6 +36,19 @@ describe('uniqBy', () => { }) }) +describe('uniq', () => { + test('removes duplicates', () => { + // Given + const array = [1, 2, 2, 3] + + // When + const got = uniq(array) + + // Then + expect(got).toEqual([1, 2, 3]) + }) +}) + describe('difference', () => { test('returns the different elements', () => { // Given diff --git a/packages/cli-kit/src/public/common/array.ts b/packages/cli-kit/src/public/common/array.ts index 938e3f1f8a..703608d2c8 100644 --- a/packages/cli-kit/src/public/common/array.ts +++ b/packages/cli-kit/src/public/common/array.ts @@ -33,6 +33,16 @@ export function getArrayContainsDuplicates(array: T[]): boolean { return array.length !== new Set(array).size } +/** + * Removes duplicated items from an array. + * + * @param array - The array to inspect. + * @returns Returns the new duplicate free array. + */ +export function uniq(array: T[]): T[] { + return Array.from(new Set(array)) +} + /** * This method is like `_.uniq` except that it accepts `iteratee` which is * invoked for each element in `array` to generate the criterion by which