Skip to content

Commit

Permalink
Do not allow duplicated compliance topics
Browse files Browse the repository at this point in the history
  • Loading branch information
gonzaloriestra authored and gracejychang committed Mar 1, 2024
1 parent 0fbfb20 commit 630abd8
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 117 deletions.
110 changes: 16 additions & 94 deletions packages/app/src/cli/models/app/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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

Expand All @@ -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]
Expand All @@ -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)
}
}
}
9 changes: 8 additions & 1 deletion packages/app/src/cli/services/app/config/link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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({
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export async function ensureDevContext(
selectedApp.apiKey,
developerPlatformClient,
specifications,
selectedApp.betas,
selectedApp.flags,
),
}

Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/info.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: []},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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<OrganizationStore[]> {
Expand Down
15 changes: 14 additions & 1 deletion packages/cli-kit/src/public/common/array.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions packages/cli-kit/src/public/common/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ export function getArrayContainsDuplicates<T>(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<T>(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
Expand Down

0 comments on commit 630abd8

Please sign in to comment.