diff --git a/.changeset/tasty-guests-fry.md b/.changeset/tasty-guests-fry.md new file mode 100644 index 0000000000..6da7abd802 --- /dev/null +++ b/.changeset/tasty-guests-fry.md @@ -0,0 +1,5 @@ +--- +'@directus/api': patch +--- + +Ensured that policies attached to a user, role and parent roles are correctly prioritized diff --git a/api/src/permissions/lib/fetch-policies.test.ts b/api/src/permissions/lib/fetch-policies.test.ts index 463a95ab07..e30d4439f9 100644 --- a/api/src/permissions/lib/fetch-policies.test.ts +++ b/api/src/permissions/lib/fetch-policies.test.ts @@ -1,9 +1,8 @@ import type { Accountability } from '@directus/types'; import { beforeEach, expect, test, vi } from 'vitest'; import { AccessService } from '../../services/access.js'; -import type { AccessRow } from '../modules/process-ast/types.js'; import type { Context } from '../types.js'; -import { _fetchPolicies as fetchPolicies } from './fetch-policies.js'; +import { _fetchPolicies as fetchPolicies, type AccessRow } from './fetch-policies.js'; vi.mock('../../services/access.js', () => ({ AccessService: vi.fn(), @@ -42,7 +41,7 @@ test('Fetches policies for public role and user when user is given without role' }, ], }, - fields: ['policy.id', 'policy.ip_access'], + fields: ['policy.id', 'policy.ip_access', 'role'], limit: -1, }); @@ -69,7 +68,7 @@ test('Fetches policies for public role when no roles and user are given', async }, ], }, - fields: ['policy.id', 'policy.ip_access'], + fields: ['policy.id', 'policy.ip_access', 'role'], limit: -1, }); @@ -87,7 +86,7 @@ test('Fetched policies for user roles', async () => { _in: ['role-a', 'role-b'], }, }, - fields: ['policy.id', 'policy.ip_access'], + fields: ['policy.id', 'policy.ip_access', 'role'], limit: -1, }); @@ -114,7 +113,7 @@ test('Fetches policies for user roles and user if user is passed', async () => { }, ], }, - fields: ['policy.id', 'policy.ip_access'], + fields: ['policy.id', 'policy.ip_access', 'role'], limit: -1, }); @@ -130,12 +129,14 @@ test('Filters policies based on ip access on access row', async () => { id: 'policy-a', ip_access: ['127.0.0.0/29'], }, + role: null, }, { policy: { id: 'policy-b', ip_access: ['1.1.1.1/32'], }, + role: null, }, ); @@ -143,3 +144,42 @@ test('Filters policies based on ip access on access row', async () => { expect(policies).toEqual(['policy-a']); }); + +test('Sorts policies by priority', async () => { + const acc = { roles: ['role-a', 'role-b'], user: 'user-a' } as unknown as Accountability; + + rows.push( + { + policy: { + id: 'policy-c', + ip_access: null, + }, + role: null, + }, + { + policy: { + id: 'policy-d', + ip_access: null, + }, + role: null, + }, + { + policy: { + id: 'policy-b', + ip_access: null, + }, + role: 'role-b', + }, + { + policy: { + id: 'policy-a', + ip_access: null, + }, + role: 'role-a', + }, + ); + + const policies = await fetchPolicies(acc, {} as Context); + + expect(policies).toEqual(['policy-a', 'policy-b', 'policy-c', 'policy-d']); +}); diff --git a/api/src/permissions/lib/fetch-policies.ts b/api/src/permissions/lib/fetch-policies.ts index c729a53ee5..3aa3808d47 100644 --- a/api/src/permissions/lib/fetch-policies.ts +++ b/api/src/permissions/lib/fetch-policies.ts @@ -1,8 +1,12 @@ import type { Accountability, Filter } from '@directus/types'; -import type { AccessRow } from '../modules/process-ast/types.js'; +import type { Context } from '../types.js'; import { filterPoliciesByIp } from '../utils/filter-policies-by-ip.js'; import { withCache } from '../utils/with-cache.js'; -import type { Context } from '../types.js'; + +export interface AccessRow { + policy: { id: string; ip_access: string[] | null }; + role: string | null; +} export const fetchPolicies = withCache('policies', _fetchPolicies, ({ roles, user, ip }) => ({ roles, user, ip })); @@ -30,11 +34,26 @@ export async function _fetchPolicies( const accessRows = (await accessService.readByQuery({ filter, - fields: ['policy.id', 'policy.ip_access'], + fields: ['policy.id', 'policy.ip_access', 'role'], limit: -1, })) as AccessRow[]; const filteredAccessRows = filterPoliciesByIp(accessRows, ip); + + /* + * Sort rows by priority (goes bottom up): + * - Parent role policies + * - Child role policies + * - User policies + */ + filteredAccessRows.sort((a, b) => { + if (!a.role && !b.role) return 0; + if (!a.role) return 1; + if (!b.role) return -1; + + return roles.indexOf(a.role) - roles.indexOf(b.role); + }); + const ids = filteredAccessRows.map(({ policy }) => policy.id); return ids; diff --git a/api/src/permissions/modules/process-ast/types.ts b/api/src/permissions/modules/process-ast/types.ts index 3f965fd18a..2873a9156a 100644 --- a/api/src/permissions/modules/process-ast/types.ts +++ b/api/src/permissions/modules/process-ast/types.ts @@ -15,7 +15,3 @@ export type FieldMap = { read: FieldMapEntries; other: FieldMapEntries; }; - -export interface AccessRow { - policy: { id: string; ip_access: string[] | null }; -} diff --git a/api/src/permissions/utils/filter-policies-by-ip.ts b/api/src/permissions/utils/filter-policies-by-ip.ts index f153a4469e..1c34910456 100644 --- a/api/src/permissions/utils/filter-policies-by-ip.ts +++ b/api/src/permissions/utils/filter-policies-by-ip.ts @@ -1,5 +1,5 @@ import { ipInNetworks } from '../../utils/ip-in-networks.js'; -import type { AccessRow } from '../modules/process-ast/types.js'; +import type { AccessRow } from '../lib/fetch-policies.js'; export function filterPoliciesByIp(policies: AccessRow[], ip: string | null | undefined) { return policies.filter(({ policy }) => {