Skip to content

Commit 37deb8f

Browse files
committed
fix(perms): scope-aware filter sanitisation for translation sub-fields (I-1)
Previously `sanitizeFilter` only checked the root field of each filter clause, so a search/filter like { translations: { _some: { description: { _icontains: 'foo' } } } } slipped through unchanged when the user had read on `translations` but not on the `description` sub-field of the junction collection. The API then rejected the query with 403 and the layout showed an empty table — exactly the symptom Bug E was supposed to eliminate. Fix: extend `sanitizeFilter` with a `nestedScopes` option mapping parent-field-name → nested-collection-name. The walker now: - treats _some / _none / _every as relation-match operators that recurse with the same scope (we are already inside the relation) - recurses into the value of a configured nested-scope field with the nested collection set as scope - calls canRead(field, scope) so the caller can dispatch to the right collection's permissions Caller in super-table.vue passes { nestedScopes: { translations: <junction collection> } } and uses (field, scope) => permissions.canRead(scope ?? collection, field). Tests: - 6 baseline tests still pass (canRead's optional second arg is backward-compatible). - 7 new tests cover the nested cases: _some/_none/_every, mixed siblings (one allowed one denied), translation branch dropped inside _or, no-options backward-compat, and the trivial case where filter never touches the nested scope. Live verification: as the test user, with description read removed from the junction collection and the bookmark filter set to {translations: {_some: {description: {_icontains: 'test'}}}}, the items query is now sent without any filter clause and returns 200 OK with all three items, instead of 403 and an empty table. Surfaced by code review on PR #57.
1 parent 7c9826f commit 37deb8f

3 files changed

Lines changed: 180 additions & 14 deletions

File tree

src/super-table.vue

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -921,7 +921,21 @@ const sanitizedFilterResult = computed(() => {
921921
filters.length === 0 ? undefined : filters.length === 1 ? filters[0] : { _and: filters };
922922
if (!merged) return { sanitized: undefined, removed: [] as string[] };
923923
924-
return sanitizeFilter(merged, (field: string) => permissions.canRead(collection.value, field));
924+
// Permission check is scope-aware: when the walker descends into the
925+
// translations junction (via `_some`/`_none`/`_every`), `scope` will be
926+
// the junction collection name and we check sub-field permissions there
927+
// instead of on the parent collection. Without this, a filter like
928+
// `{ translations: { _some: { description: ... } } }` would slip past
929+
// the parent `canRead('translations')` check and hit the server with a
930+
// sub-field the user can't read — re-opening Bug E for nested filters.
931+
const nestedScopes: Record<string, string | undefined> = {
932+
translations: translationsCollectionRef.value ?? undefined,
933+
};
934+
return sanitizeFilter(
935+
merged,
936+
(field, scope) => permissions.canRead(scope ?? collection.value, field),
937+
{ nestedScopes }
938+
);
925939
});
926940
927941
const combinedFilter = computed(() => (sanitizedFilterResult.value.sanitized ?? undefined) as any);

src/utils/sanitizeFilter.ts

Lines changed: 79 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,43 +5,109 @@ export interface SanitizeResult {
55
removed: string[];
66
}
77

8+
export interface SanitizeFilterOptions {
9+
/**
10+
* Map from a parent-field name to the collection it nests into.
11+
* When the walker traverses into the value of a field listed here,
12+
* inner field names will be checked against the nested collection's
13+
* permissions instead of the outer scope.
14+
*
15+
* Example: `{ translations: 'pages_translations' }`
16+
* - Top-level `translations` is checked against the parent collection.
17+
* - Sub-fields inside relation match operators (`_some`, `_none`, `_every`)
18+
* are checked against `pages_translations`.
19+
*/
20+
nestedScopes?: Record<string, string | undefined>;
21+
}
22+
823
const LOGICAL_KEYS = new Set(['_and', '_or']);
24+
const RELATION_MATCH_OPS = new Set(['_some', '_none', '_every']);
925

26+
/**
27+
* Walk a Directus filter tree and remove conditions that reference fields
28+
* the caller can't read. Logical operators (`_and`, `_or`) collapse if all
29+
* their branches are dropped. Relation match operators (`_some`, `_none`,
30+
* `_every`) recurse into a nested scope when configured via `nestedScopes`,
31+
* so e.g. `{ translations: { _some: { description: ... } } }` is checked
32+
* both at the parent level (translations field readable?) and at the
33+
* junction collection level (description field readable?).
34+
*
35+
* The `canRead` callback receives `(field, scope?)`. When `scope` is
36+
* undefined, the caller should check against the outer/parent collection.
37+
* When `scope` is set (matches a value from `nestedScopes`), the caller
38+
* should check against that collection.
39+
*/
1040
export function sanitizeFilter(
1141
filter: FilterValue,
12-
canRead: (field: string) => boolean
42+
canRead: (field: string, scope?: string) => boolean,
43+
options: SanitizeFilterOptions = {}
1344
): SanitizeResult {
1445
const removed: string[] = [];
46+
const nestedScopes = options.nestedScopes ?? {};
47+
48+
function isEmpty(node: FilterValue): boolean {
49+
if (node === null || node === undefined) return true;
50+
if (Array.isArray(node)) return node.length === 0;
51+
if (typeof node === 'object') return Object.keys(node).length === 0;
52+
return false;
53+
}
1554

16-
function walk(node: FilterValue): FilterValue {
55+
function walk(node: FilterValue, scope?: string): FilterValue {
1756
if (node === null || node === undefined || typeof node !== 'object') return node;
1857

1958
if (Array.isArray(node)) {
20-
const cleaned = node
21-
.map((entry) => walk(entry as FilterValue))
22-
.filter((entry) => {
23-
if (entry === null) return false;
24-
if (typeof entry === 'object' && !Array.isArray(entry) && Object.keys(entry).length === 0)
25-
return false;
26-
return true;
27-
});
28-
return cleaned;
59+
return node
60+
.map((entry) => walk(entry as FilterValue, scope))
61+
.filter((entry) => !isEmpty(entry as FilterValue));
2962
}
3063

3164
const out: Record<string, unknown> = {};
3265
for (const [key, value] of Object.entries(node)) {
66+
// Logical: `_and` / `_or` — recurse into the array, same scope.
3367
if (LOGICAL_KEYS.has(key)) {
34-
const cleaned = walk(value as FilterValue);
68+
const cleaned = walk(value as FilterValue, scope);
3569
if (Array.isArray(cleaned) && cleaned.length > 0) out[key] = cleaned;
3670
continue;
3771
}
3872

73+
// Relation match: `_some` / `_none` / `_every` — recurse into the
74+
// single inner filter object, same scope (we are already inside the
75+
// relation; the parent established the scope before calling us).
76+
if (RELATION_MATCH_OPS.has(key)) {
77+
const cleaned = walk(value as FilterValue, scope);
78+
if (!isEmpty(cleaned)) out[key] = cleaned;
79+
continue;
80+
}
81+
82+
// Other operators (`_eq`, `_icontains`, etc.) — leaf, passthrough.
83+
if (key.startsWith('_')) {
84+
out[key] = value;
85+
continue;
86+
}
87+
88+
// Field name — check permission at the current scope.
3989
const rootField = key.split('.')[0];
40-
if (!canRead(rootField)) {
90+
if (!canRead(rootField, scope)) {
4191
removed.push(key);
4292
continue;
4393
}
4494

95+
// Field is allowed at this scope. Does traversing into its value
96+
// open a nested scope (e.g. translations → pages_translations)?
97+
const nestedScope = nestedScopes[rootField];
98+
if (nestedScope && value && typeof value === 'object' && !Array.isArray(value)) {
99+
const cleanedInner = walk(value as FilterValue, nestedScope);
100+
if (!isEmpty(cleanedInner)) {
101+
out[key] = cleanedInner;
102+
}
103+
// If the inner walk dropped everything, the parent's clause is
104+
// implicitly removed too. We do NOT push the parent to `removed[]`
105+
// because the parent itself was permitted — the user-visible
106+
// "removed fields" list contains the actual sub-fields that lost
107+
// access (already pushed by the inner walk).
108+
continue;
109+
}
110+
45111
out[key] = value;
46112
}
47113

tests/unit/utils/sanitizeFilter.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,89 @@ describe('sanitizeFilter', () => {
4343
});
4444
});
4545
});
46+
47+
describe('sanitizeFilter with nestedScopes (translation sub-fields)', () => {
48+
// Caller mirrors the real super-table.vue wiring: parent collection is
49+
// 'pages'; the translations junction is 'pages_translations'. The user
50+
// can read `text` in the junction but not `description`.
51+
const canRead = (field: string, scope?: string) => {
52+
if (scope === 'pages_translations') return field === 'text';
53+
return ['title', 'translations', 'id'].includes(field);
54+
};
55+
const opts = { nestedScopes: { translations: 'pages_translations' } };
56+
57+
it('keeps allowed sub-field inside _some on translations', () => {
58+
const filter = { translations: { _some: { text: { _icontains: 'foo' } } } };
59+
expect(sanitizeFilter(filter, canRead, opts)).toEqual({
60+
sanitized: { translations: { _some: { text: { _icontains: 'foo' } } } },
61+
removed: [],
62+
});
63+
});
64+
65+
it('drops disallowed sub-field inside _some, dropping the whole translations clause', () => {
66+
const filter = { translations: { _some: { description: { _icontains: 'foo' } } } };
67+
expect(sanitizeFilter(filter, canRead, opts)).toEqual({
68+
sanitized: null,
69+
removed: ['description'],
70+
});
71+
});
72+
73+
it('keeps allowed sibling and drops disallowed sibling inside _some', () => {
74+
const filter = {
75+
translations: {
76+
_some: {
77+
text: { _icontains: 'foo' },
78+
description: { _icontains: 'bar' },
79+
},
80+
},
81+
};
82+
expect(sanitizeFilter(filter, canRead, opts)).toEqual({
83+
sanitized: { translations: { _some: { text: { _icontains: 'foo' } } } },
84+
removed: ['description'],
85+
});
86+
});
87+
88+
it('drops translation branch inside _or while keeping accessible siblings', () => {
89+
const filter = {
90+
_or: [
91+
{ translations: { _some: { description: { _icontains: 'foo' } } } },
92+
{ title: { _icontains: 'foo' } },
93+
],
94+
};
95+
expect(sanitizeFilter(filter, canRead, opts)).toEqual({
96+
sanitized: { _or: [{ title: { _icontains: 'foo' } }] },
97+
removed: ['description'],
98+
});
99+
});
100+
101+
it('handles _none and _every relation match operators', () => {
102+
const filter = {
103+
_and: [
104+
{ translations: { _none: { description: { _eq: 'spam' } } } },
105+
{ translations: { _every: { text: { _nnull: true } } } },
106+
],
107+
};
108+
expect(sanitizeFilter(filter, canRead, opts)).toEqual({
109+
sanitized: { _and: [{ translations: { _every: { text: { _nnull: true } } } }] },
110+
removed: ['description'],
111+
});
112+
});
113+
114+
it('passes through when no nestedScopes configured (backward compatibility)', () => {
115+
// Without nestedScopes, the walker treats translations as a leaf field.
116+
// Permission only checks the parent-level field, not sub-fields.
117+
const filter = { translations: { _some: { description: { _icontains: 'foo' } } } };
118+
expect(sanitizeFilter(filter, canRead)).toEqual({
119+
sanitized: filter,
120+
removed: [],
121+
});
122+
});
123+
124+
it('does not require translations scope when filter touches only parent fields', () => {
125+
const filter = { _and: [{ title: { _eq: 'foo' } }, { id: { _eq: 1 } }] };
126+
expect(sanitizeFilter(filter, canRead, opts)).toEqual({
127+
sanitized: filter,
128+
removed: [],
129+
});
130+
});
131+
});

0 commit comments

Comments
 (0)