diff --git a/packages/cli/src/lib/program.ts b/packages/cli/src/lib/program.ts index 6230e0f1..fbbb1e46 100644 --- a/packages/cli/src/lib/program.ts +++ b/packages/cli/src/lib/program.ts @@ -38,6 +38,9 @@ function cleanCommandOptions(commandOptions: Record) { if (commandOptions.specs === true) { delete commandOptions.specs; } + if (commandOptions.syncPolicyRoles === true) { + delete commandOptions.syncPolicyRoles; + } return commandOptions; } @@ -142,6 +145,10 @@ export function createProgram() { '--no-collections', `should pull and push the collections (default "${DefaultConfig.collections}")`, ); + const noSyncPolicyRolesOption = new Option( + '--no-sync-policy-roles', + `should sync the role ↔ policy attachments (directus_access entries linking roles and policies). Disable to leave existing role-policy assignments on the target untouched (default "${DefaultConfig.syncPolicyRoles}")`, + ); const preserveIdsOption = new Option( '--preserve-ids ', `comma separated list of collections that preserve their original ids (default to none). Use "*" or "all" to preserve all ids, if applicable.`, @@ -197,6 +204,7 @@ export function createProgram() { .addOption(excludeCollectionsOption) .addOption(onlyCollectionsOption) .addOption(noCollectionsOption) + .addOption(noSyncPolicyRolesOption) .addOption(preserveIdsOption) .addOption(snapshotPathOption) .addOption(noSnapshotOption) @@ -215,6 +223,7 @@ export function createProgram() { .addOption(excludeCollectionsOption) .addOption(onlyCollectionsOption) .addOption(noCollectionsOption) + .addOption(noSyncPolicyRolesOption) .addOption(snapshotPathOption) .addOption(noSnapshotOption) .addOption(noSplitOption) @@ -230,6 +239,7 @@ export function createProgram() { .addOption(excludeCollectionsOption) .addOption(onlyCollectionsOption) .addOption(noCollectionsOption) + .addOption(noSyncPolicyRolesOption) .addOption(preserveIdsOption) .addOption(snapshotPathOption) .addOption(noSnapshotOption) diff --git a/packages/cli/src/lib/services/collections/policies/data-client.ts b/packages/cli/src/lib/services/collections/policies/data-client.ts index fe0cd179..091d0a00 100644 --- a/packages/cli/src/lib/services/collections/policies/data-client.ts +++ b/packages/cli/src/lib/services/collections/policies/data-client.ts @@ -17,10 +17,15 @@ import { import { LoggerService } from '../../logger'; import { POLICIES_COLLECTION } from './constants'; import deepmerge from 'deepmerge'; +import { ConfigService } from '../../config'; @Service() export class PoliciesDataClient extends DataClient { - constructor(loggerService: LoggerService, migrationClient: MigrationClient) { + constructor( + loggerService: LoggerService, + migrationClient: MigrationClient, + protected readonly config: ConfigService, + ) { super(loggerService.getChild(POLICIES_COLLECTION), migrationClient); } @@ -33,9 +38,15 @@ export class PoliciesDataClient extends DataClient { } protected getQueryCommand(query: Query) { + // When role-policy attachments sync is disabled, omit the roles fields + // entirely from the dump so they are neither tracked nor diffed. + // See https://github.com/tractr/directus-sync/issues/199 + const extraFields = this.config.shouldSyncPolicyRoles() + ? ['*', 'roles.role', 'roles.sort'] + : ['*']; return readPolicies( deepmerge>(query, { - fields: ['*', 'roles.role', 'roles.sort'], + fields: extraFields, }), ); } @@ -44,6 +55,13 @@ export class PoliciesDataClient extends DataClient { itemId: string, diffItem: Partial>, ) { + // When role-policy attachments sync is disabled, drop the roles diff + // entirely so existing attachments on the target are left untouched. + // See https://github.com/tractr/directus-sync/issues/199 + if (!this.config.shouldSyncPolicyRoles() && diffItem.roles) { + const { roles: _ignored, ...rest } = diffItem as Partial; + diffItem = rest as Partial>; + } // Explicit update of the roles field (many-to-many relation) // Issue : https://github.com/tractr/directus-sync/issues/148 if (diffItem.roles) { diff --git a/packages/cli/src/lib/services/collections/policies/data-mapper.ts b/packages/cli/src/lib/services/collections/policies/data-mapper.ts index 243880d2..3cf1e519 100644 --- a/packages/cli/src/lib/services/collections/policies/data-mapper.ts +++ b/packages/cli/src/lib/services/collections/policies/data-mapper.ts @@ -4,18 +4,27 @@ import { LoggerService } from '../../logger'; import { POLICIES_COLLECTION } from './constants'; import { DirectusPolicy } from './interfaces'; import { RolesIdMapperClient } from '../roles'; +import { ConfigService } from '../../config'; @Service() export class PoliciesDataMapper extends DataMapper { - protected fieldsToIgnore: Field[] = ['users', 'permissions']; - protected idMappers: IdMappers = { - roles: { - // @ts-expect-error TODO: Bad SDK Typing - role: Container.get(RolesIdMapperClient), - }, - }; + protected fieldsToIgnore: Field[]; + protected idMappers: IdMappers; - constructor(loggerService: LoggerService) { + constructor(loggerService: LoggerService, config: ConfigService) { super(loggerService.getChild(POLICIES_COLLECTION)); + + if (config.shouldSyncPolicyRoles()) { + this.fieldsToIgnore = ['users', 'permissions']; + this.idMappers = { + roles: { + // @ts-expect-error TODO: Bad SDK Typing + role: Container.get(RolesIdMapperClient), + }, + }; + } else { + this.fieldsToIgnore = ['users', 'permissions', 'roles']; + this.idMappers = {}; + } } } diff --git a/packages/cli/src/lib/services/config/config.ts b/packages/cli/src/lib/services/config/config.ts index 58ef3e81..47bfba27 100644 --- a/packages/cli/src/lib/services/config/config.ts +++ b/packages/cli/src/lib/services/config/config.ts @@ -177,6 +177,11 @@ export class ConfigService { return list.filter((collection) => !exclude.includes(collection)); } + @Cacheable() + shouldSyncPolicyRoles() { + return this.requireOptions('syncPolicyRoles'); + } + @Cacheable() shouldPreserveIds(collection: CollectionPreservableIdName) { const preserveIds = this.requireOptions('preserveIds'); diff --git a/packages/cli/src/lib/services/config/default-config.ts b/packages/cli/src/lib/services/config/default-config.ts index 76c904ab..854ffdc4 100644 --- a/packages/cli/src/lib/services/config/default-config.ts +++ b/packages/cli/src/lib/services/config/default-config.ts @@ -17,6 +17,7 @@ export const DefaultConfig: Pick< | 'excludeCollections' | 'onlyCollections' | 'collections' + | 'syncPolicyRoles' | 'preserveIds' | 'snapshotPath' | 'snapshot' @@ -42,6 +43,7 @@ export const DefaultConfig: Pick< excludeCollections: [], onlyCollections: [], collections: true, + syncPolicyRoles: true, preserveIds: [], // Snapshot snapshotPath: 'snapshot', diff --git a/packages/cli/src/lib/services/config/schema.ts b/packages/cli/src/lib/services/config/schema.ts index da7e71e7..995c25fe 100644 --- a/packages/cli/src/lib/services/config/schema.ts +++ b/packages/cli/src/lib/services/config/schema.ts @@ -106,6 +106,7 @@ export const OptionsFields = { excludeCollections: z.array(CollectionEnum).optional(), onlyCollections: z.array(CollectionEnum).optional(), collections: z.boolean(), + syncPolicyRoles: z.boolean(), preserveIds: z.union([ z.array(CollectionPreservableIdEnum).optional(), z.enum(['all', '*']), @@ -162,6 +163,7 @@ export const ConfigFileOptionsSchema = z.object({ excludeCollections: OptionsFields.excludeCollections.optional(), onlyCollections: OptionsFields.onlyCollections.optional(), collections: OptionsFields.collections.optional(), + syncPolicyRoles: OptionsFields.syncPolicyRoles.optional(), preserveIds: OptionsFields.preserveIds.optional(), // Snapshot config snapshotPath: OptionsFields.snapshotPath.optional(), diff --git a/packages/e2e/spec/entrypoint.spec.ts b/packages/e2e/spec/entrypoint.spec.ts index b749355d..7143d20e 100644 --- a/packages/e2e/spec/entrypoint.spec.ts +++ b/packages/e2e/spec/entrypoint.spec.ts @@ -15,6 +15,7 @@ import { pushWithExistingUuid, pushWithUserPolicyAssignment, pushWithRolePolicyAssignmentChanges, + pushWithNoSyncPolicyRoles, prettyDiffOutput, } from './pull-diff-push/index.js'; import { sortJson } from './pull-diff-push/sort-json.js'; @@ -88,6 +89,7 @@ describe('Tests entrypoint ->', () => { pushWithExistingUuid(context); pushWithUserPolicyAssignment(context); pushWithRolePolicyAssignmentChanges(context); + pushWithNoSyncPolicyRoles(context); updateDefaultData(context); pushWithDependencies(context); diff --git a/packages/e2e/spec/pull-diff-push/index.ts b/packages/e2e/spec/pull-diff-push/index.ts index 4e2ca762..ddb9e97a 100644 --- a/packages/e2e/spec/pull-diff-push/index.ts +++ b/packages/e2e/spec/pull-diff-push/index.ts @@ -11,5 +11,6 @@ export * from './push-twice-on-empty-instance.js'; export * from './push-with-existing-uuid.js'; export * from './push-with-user-policy-assignment.js'; export * from './push-with-role-policy-assignment-changes.js'; +export * from './push-with-no-sync-policy-roles.js'; export * from './sort-json.js'; export * from './pretty-diff-output.js'; diff --git a/packages/e2e/spec/pull-diff-push/push-with-no-sync-policy-roles.ts b/packages/e2e/spec/pull-diff-push/push-with-no-sync-policy-roles.ts new file mode 100644 index 00000000..3dacdab8 --- /dev/null +++ b/packages/e2e/spec/pull-diff-push/push-with-no-sync-policy-roles.ts @@ -0,0 +1,113 @@ +import { + createPolicy, + DirectusPolicy, + readPolicy, + updatePolicy, +} from '@directus/sdk'; +import { + Context, + getDumpedSystemCollectionsContents, + getPolicy, + newRole, + Schema, +} from '../helpers/index.js'; + +/** + * Regression test for https://github.com/tractr/directus-sync/issues/199 + * + * When `--no-sync-policy-roles` is passed, role ↔ policy attachments + * (directus_access entries) must be left untouched on the target, + * even when the local dump's policies have a different set of attachments. + */ +export const pushWithNoSyncPolicyRoles = (context: Context) => { + it('should leave existing role-policy attachments untouched when --no-sync-policy-roles is set', async () => { + const sync = await context.getSync('temp/push-with-no-sync-policy-roles'); + const directus = context.getDirectus(); + const client = directus.get(); + + const role = (roleId: string) => (access: { role: string }) => + access.role === roleId; + + // Local roles: role1 + role2 attached to the policy + const role1 = await newRole(client); + const role2 = await newRole(client); + + const policy = await client.request( + createPolicy({ + ...getPolicy(null), + roles: [ + { role: role1.id, sort: 1 }, + { role: role2.id, sort: 2 }, + ], + } as unknown as DirectusPolicy), + ); + + // Pull the current state (with attachments) using the default behavior + await sync.pull(); + + // Simulate "production": a third role gets attached and the original + // role1 attachment is removed by an admin on the target instance. + const role3 = await newRole(client); + const currentPolicy = await client.request( + readPolicy(policy.id, { fields: ['id', 'roles.*'] }), + ); + const role1Access = currentPolicy.roles.find(role(role1.id)); + await client.request( + updatePolicy(policy.id, { + roles: { + create: [{ role: role3.id, sort: 3 }], + update: [], + delete: [role1Access.id], + }, + } as unknown as DirectusPolicy), + ); + + // Sanity check: target now has role2 + role3 (no role1) + const beforePush = await client.request( + readPolicy(policy.id, { fields: ['id', 'roles.*'] }), + ); + expect(beforePush.roles.length).toBe(2); + expect(beforePush.roles.find(role(role1.id))).toBeUndefined(); + expect(beforePush.roles.find(role(role2.id))).toBeDefined(); + expect(beforePush.roles.find(role(role3.id))).toBeDefined(); + + // Push with --no-sync-policy-roles: target attachments must be preserved + await sync.push(['--no-sync-policy-roles']); + + const afterPush = await client.request( + readPolicy(policy.id, { fields: ['id', 'roles.*'] }), + ); + // role1 must NOT have been re-added (local dump still has it, but flag is on) + expect(afterPush.roles.find(role(role1.id))).toBeUndefined(); + // role3 must NOT have been removed (it's only on the target, not in dump) + expect(afterPush.roles.find(role(role3.id))).toBeDefined(); + // role2 must still be there + expect(afterPush.roles.find(role(role2.id))).toBeDefined(); + expect(afterPush.roles.length).toBe(2); + }); + + it('should not dump role attachments on pull when --no-sync-policy-roles is set', async () => { + const sync = await context.getSync('temp/pull-with-no-sync-policy-roles'); + const directus = context.getDirectus(); + const client = directus.get(); + + const role1 = await newRole(client); + const policy = await client.request( + createPolicy({ + ...getPolicy(null), + roles: [{ role: role1.id, sort: 1 }], + } as unknown as DirectusPolicy), + ); + + await sync.pull(['--no-sync-policy-roles']); + + // Read back the dumped policy file and assert no roles attachments + const { policies } = getDumpedSystemCollectionsContents(sync.getDumpPath()); + const ourPolicy = policies?.find( + (p: { name?: string }) => p.name === policy.name, + ); + expect(ourPolicy).toBeDefined(); + // roles must be absent or empty (we did not query them) + expect(ourPolicy?.roles ?? []).toEqual([]); + }); +}; diff --git a/website/docs/features/configuration.mdx b/website/docs/features/configuration.mdx index f33083e7..a9d8ae8e 100644 --- a/website/docs/features/configuration.mdx +++ b/website/docs/features/configuration.mdx @@ -62,6 +62,12 @@ These options can be used with any command to configure the operation of `direct - `--no-collections` Do not pull and push the Directus collections. By default, the collections are pulled and pushed. +- `--no-sync-policy-roles` + Do not sync the role ↔ policy attachments (`directus_access` entries linking roles and policies). When set, + these attachments are neither dumped on `pull` nor modified on `push`, leaving existing assignments on the + target instance untouched. Useful when end-users manage policy assignments directly in production. + See [#199](https://github.com/tractr/directus-sync/issues/199). + - `--preserve-ids ` Comma-separated list of directus collections to preserve the original ids during the `pull` or `push` process. Possible collections are: `dashboards`, `operations`, `panels`, `policies`, `roles` and `translations`. @@ -125,6 +131,7 @@ module.exports = { onlyCollections: ['roles', 'policies', 'permissions', 'settings'], collections: true, excludeCollections: ['settings'], + syncPolicyRoles: true, // set to false to leave role↔policy attachments untouched on the target preserveIds: ['roles', 'panels'], // can be '*' or 'all' to preserve all ids, or an array of collections maxPushRetries: 20, snapshotPath: 'snapshot', diff --git a/website/docs/help-outputs/diff.md b/website/docs/help-outputs/diff.md index eede83f0..3b3a6578 100644 --- a/website/docs/help-outputs/diff.md +++ b/website/docs/help-outputs/diff.md @@ -5,6 +5,7 @@ Options: -x, --exclude-collections comma separated list of collections to exclude from the process (default to none) -o, --only-collections comma separated list of collections to include in the process (default to all) --no-collections should pull and push the collections (default "true") + --no-sync-policy-roles should sync the role ↔ policy attachments (directus_access entries linking roles and policies). Disable to leave existing role-policy assignments on the target untouched (default "true") --snapshot-path the path for the schema snapshot dump, relative to the dump path (default "snapshot") --no-snapshot should pull and push the Directus schema (default "true") --no-split should split the schema snapshot into multiple files (default "true") diff --git a/website/docs/help-outputs/pull.md b/website/docs/help-outputs/pull.md index 92438600..90069605 100644 --- a/website/docs/help-outputs/pull.md +++ b/website/docs/help-outputs/pull.md @@ -5,6 +5,7 @@ Options: -x, --exclude-collections comma separated list of collections to exclude from the process (default to none) -o, --only-collections comma separated list of collections to include in the process (default to all) --no-collections should pull and push the collections (default "true") + --no-sync-policy-roles should sync the role ↔ policy attachments (directus_access entries linking roles and policies). Disable to leave existing role-policy assignments on the target untouched (default "true") --preserve-ids comma separated list of collections that preserve their original ids (default to none). Use "*" or "all" to preserve all ids, if applicable. --snapshot-path the path for the schema snapshot dump, relative to the dump path (default "snapshot") --no-snapshot should pull and push the Directus schema (default "true") diff --git a/website/docs/help-outputs/push.md b/website/docs/help-outputs/push.md index 6f7e7a67..513f9708 100644 --- a/website/docs/help-outputs/push.md +++ b/website/docs/help-outputs/push.md @@ -5,6 +5,7 @@ Options: -x, --exclude-collections comma separated list of collections to exclude from the process (default to none) -o, --only-collections comma separated list of collections to include in the process (default to all) --no-collections should pull and push the collections (default "true") + --no-sync-policy-roles should sync the role ↔ policy attachments (directus_access entries linking roles and policies). Disable to leave existing role-policy assignments on the target untouched (default "true") --preserve-ids comma separated list of collections that preserve their original ids (default to none). Use "*" or "all" to preserve all ids, if applicable. --snapshot-path the path for the schema snapshot dump, relative to the dump path (default "snapshot") --no-snapshot should pull and push the Directus schema (default "true")