Skip to content

Commit cbbf98e

Browse files
authored
fix(plugin-multi-tenant): ensure relationship filter filters based on doc.tenant (#13925)
Previously, relationship fields were only filtered based on the `payload-tenant` cookie - if the relationship points to a relation where `doc.relation.tenant !== cookies.get('payload-tenant')`, it will fail validation. This is good! However, if no headers are present (e.g. when using the local API to create or update a document), this validation will pass, even if the document belongs to a different tenant. The following test is passing in this PR and failing in main: `ensure relationship document with relationship to different tenant cannot be created even if no tenant header passed`. This PR extends the validation logic to respect the tenant stored in the document's data and only read the headers if the document does not have a tenant set yet. Old logic: `doc.relation.tenant !== cookies.get('payload-tenant')` => fail validation New logic: `doc.relation.tenant !== doc.tenant ?? cookies.get('payload-tenant')` => fail validation --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211456244666493
1 parent f39c7bf commit cbbf98e

File tree

7 files changed

+203
-26
lines changed

7 files changed

+203
-26
lines changed

packages/plugin-multi-tenant/src/filters/filterDocumentsByTenants.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import { getTenantFromCookie } from '../utilities/getTenantFromCookie.js'
88
import { getUserTenantIDs } from '../utilities/getUserTenantIDs.js'
99

1010
type Args<ConfigType = unknown> = {
11+
/**
12+
* If the document this filter is run belongs to a tenant, the tenant ID should be passed here.
13+
* If set, this will be used instead of the tenant cookie
14+
*/
15+
docTenantID?: number | string
1116
filterFieldName: string
1217
req: PayloadRequest
1318
tenantsArrayFieldName?: string
@@ -18,6 +23,7 @@ type Args<ConfigType = unknown> = {
1823
>['userHasAccessToAllTenants']
1924
}
2025
export const filterDocumentsByTenants = <ConfigType = unknown>({
26+
docTenantID,
2127
filterFieldName,
2228
req,
2329
tenantsArrayFieldName = defaults.tenantsArrayFieldName,
@@ -31,7 +37,7 @@ export const filterDocumentsByTenants = <ConfigType = unknown>({
3137
})
3238

3339
// scope results to selected tenant
34-
const selectedTenant = getTenantFromCookie(req.headers, idType)
40+
const selectedTenant = docTenantID ?? getTenantFromCookie(req.headers, idType)
3541
if (selectedTenant) {
3642
return {
3743
[filterFieldName]: {

packages/plugin-multi-tenant/src/utilities/addFilterOptionsToFields.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ function addFilter<ConfigType = unknown>({
185185

186186
// Custom tenant filter
187187
const tenantFilterResults = filterDocumentsByTenants({
188+
docTenantID: args.data?.[tenantFieldName],
188189
filterFieldName: tenantFieldName,
189190
req: args.req,
190191
tenantsArrayFieldName,
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
export const Relationships: CollectionConfig = {
4+
slug: 'relationships',
5+
admin: {
6+
useAsTitle: 'title',
7+
group: 'Tenant Collections',
8+
},
9+
fields: [
10+
{
11+
name: 'title',
12+
type: 'text',
13+
required: true,
14+
},
15+
{
16+
name: 'relationship',
17+
type: 'relationship',
18+
relationTo: 'relationships',
19+
},
20+
],
21+
}

test/plugin-multi-tenant/config.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@ import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
1010
import { AutosaveGlobal } from './collections/AutosaveGlobal.js'
1111
import { Menu } from './collections/Menu.js'
1212
import { MenuItems } from './collections/MenuItems.js'
13+
import { Relationships } from './collections/Relationships.js'
1314
import { Tenants } from './collections/Tenants.js'
1415
import { Users } from './collections/Users/index.js'
1516
import { seed } from './seed/index.js'
1617
import { autosaveGlobalSlug, menuItemsSlug, menuSlug } from './shared.js'
1718

1819
export default buildConfigWithDefaults({
19-
collections: [Tenants, Users, MenuItems, Menu, AutosaveGlobal],
20+
collections: [Tenants, Users, MenuItems, Menu, AutosaveGlobal, Relationships],
2021
admin: {
2122
autoLogin: false,
2223
importMap: {
@@ -48,6 +49,8 @@ export default buildConfigWithDefaults({
4849
[autosaveGlobalSlug]: {
4950
isGlobal: true,
5051
},
52+
53+
['relationships']: {},
5154
},
5255
i18n: {
5356
translations: {

test/plugin-multi-tenant/int.spec.ts

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
import type { DefaultDocumentIDType, PaginatedDocs, Payload } from 'payload'
2+
13
import path from 'path'
2-
import { NotFound, type Payload } from 'payload'
3-
import { wait } from 'payload/shared'
44
import { fileURLToPath } from 'url'
55

66
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
7+
import type { Relationship } from './payload-types.js'
78

89
import { devUser } from '../credentials.js'
910
import { initPayloadInt } from '../helpers/initPayloadInt.js'
@@ -48,5 +49,89 @@ describe('@payloadcms/plugin-multi-tenant', () => {
4849

4950
expect(tenant1).toHaveProperty('id')
5051
})
52+
53+
describe('relationships', () => {
54+
let anchorBarRelationships: PaginatedDocs<Relationship>
55+
let blueDogRelationships: PaginatedDocs<Relationship>
56+
let anchorBarTenantID: DefaultDocumentIDType
57+
let blueDogTenantID: DefaultDocumentIDType
58+
59+
beforeEach(async () => {
60+
anchorBarRelationships = await payload.find({
61+
collection: 'relationships',
62+
where: {
63+
'tenant.name': {
64+
equals: 'Anchor Bar',
65+
},
66+
},
67+
})
68+
69+
blueDogRelationships = await payload.find({
70+
collection: 'relationships',
71+
where: {
72+
'tenant.name': {
73+
equals: 'Blue Dog',
74+
},
75+
},
76+
})
77+
78+
// @ts-expect-error unsafe access okay in test
79+
80+
anchorBarTenantID = anchorBarRelationships.docs[0].tenant.id
81+
// @ts-expect-error unsafe access okay in test
82+
blueDogTenantID = blueDogRelationships.docs[0].tenant.id
83+
})
84+
85+
it('ensure relationship document with relationship within same tenant can be created', async () => {
86+
const newRelationship = await payload.create({
87+
collection: 'relationships',
88+
data: {
89+
title: 'Relationship to Anchor Bar',
90+
// @ts-expect-error unsafe access okay in test
91+
relationship: anchorBarRelationships.docs[0].id,
92+
tenant: anchorBarTenantID,
93+
},
94+
req: {
95+
headers: new Headers([['cookie', `payload-tenant=${anchorBarTenantID}`]]),
96+
},
97+
})
98+
99+
// @ts-expect-error unsafe access okay in test
100+
expect(newRelationship.relationship?.title).toBe('Owned by bar with no ac')
101+
})
102+
103+
it('ensure relationship document with relationship to different tenant cannot be created if tenant header passed', async () => {
104+
await expect(
105+
payload.create({
106+
collection: 'relationships',
107+
data: {
108+
title: 'Relationship to Blue Dog',
109+
// @ts-expect-error unsafe access okay in test
110+
relationship: blueDogRelationships.docs[0].id,
111+
tenant: anchorBarTenantID,
112+
},
113+
req: {
114+
headers: new Headers([['cookie', `payload-tenant=${anchorBarTenantID}`]]),
115+
},
116+
}),
117+
).rejects.toThrow('The following field is invalid: Relationship')
118+
})
119+
120+
it('ensure relationship document with relationship to different tenant cannot be created even if no tenant header passed', async () => {
121+
// Should filter based on data.tenant instead of tenant cookie
122+
await expect(
123+
payload.create({
124+
collection: 'relationships',
125+
data: {
126+
title: 'Relationship to Blue Dog',
127+
// @ts-expect-error unsafe access okay in test
128+
relationship: blueDogRelationships.docs[0].id,
129+
tenant: anchorBarTenantID,
130+
},
131+
req: {},
132+
}),
133+
).rejects.toThrow('The following field is invalid: Relationship')
134+
})
135+
})
51136
})
52137
})

test/plugin-multi-tenant/payload-types.ts

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export interface Config {
7272
'food-items': FoodItem;
7373
'food-menu': FoodMenu;
7474
'autosave-global': AutosaveGlobal;
75+
relationships: Relationship;
7576
'payload-locked-documents': PayloadLockedDocument;
7677
'payload-preferences': PayloadPreference;
7778
'payload-migrations': PayloadMigration;
@@ -87,12 +88,13 @@ export interface Config {
8788
'food-items': FoodItemsSelect<false> | FoodItemsSelect<true>;
8889
'food-menu': FoodMenuSelect<false> | FoodMenuSelect<true>;
8990
'autosave-global': AutosaveGlobalSelect<false> | AutosaveGlobalSelect<true>;
91+
relationships: RelationshipsSelect<false> | RelationshipsSelect<true>;
9092
'payload-locked-documents': PayloadLockedDocumentsSelect<false> | PayloadLockedDocumentsSelect<true>;
9193
'payload-preferences': PayloadPreferencesSelect<false> | PayloadPreferencesSelect<true>;
9294
'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
9395
};
9496
db: {
95-
defaultIDType: string;
97+
defaultIDType: number;
9698
};
9799
globals: {};
98100
globalsSelect: {};
@@ -128,11 +130,11 @@ export interface UserAuthOperations {
128130
* via the `definition` "tenants".
129131
*/
130132
export interface Tenant {
131-
id: string;
133+
id: number;
132134
name: string;
133135
domain: string;
134136
users?: {
135-
docs?: (string | User)[];
137+
docs?: (number | User)[];
136138
hasNextPage?: boolean;
137139
totalDocs?: number;
138140
};
@@ -145,11 +147,11 @@ export interface Tenant {
145147
* via the `definition` "users".
146148
*/
147149
export interface User {
148-
id: string;
150+
id: number;
149151
roles?: ('admin' | 'user')[] | null;
150152
tenants?:
151153
| {
152-
tenant: string | Tenant;
154+
tenant: number | Tenant;
153155
id?: string | null;
154156
}[]
155157
| null;
@@ -176,8 +178,8 @@ export interface User {
176178
* via the `definition` "food-items".
177179
*/
178180
export interface FoodItem {
179-
id: string;
180-
tenant?: (string | null) | Tenant;
181+
id: number;
182+
tenant?: (number | null) | Tenant;
181183
name: string;
182184
content?: {
183185
root: {
@@ -202,16 +204,16 @@ export interface FoodItem {
202204
* via the `definition` "food-menu".
203205
*/
204206
export interface FoodMenu {
205-
id: string;
206-
tenant?: (string | null) | Tenant;
207+
id: number;
208+
tenant?: (number | null) | Tenant;
207209
title: string;
208210
description?: string | null;
209211
menuItems?:
210212
| {
211213
/**
212214
* Automatically filtered by selected tenant
213215
*/
214-
menuItem: string | FoodItem;
216+
menuItem: number | FoodItem;
215217
active?: boolean | null;
216218
id?: string | null;
217219
}[]
@@ -224,45 +226,61 @@ export interface FoodMenu {
224226
* via the `definition` "autosave-global".
225227
*/
226228
export interface AutosaveGlobal {
227-
id: string;
228-
tenant?: (string | null) | Tenant;
229+
id: number;
230+
tenant?: (number | null) | Tenant;
229231
title: string;
230232
description?: string | null;
231233
updatedAt: string;
232234
createdAt: string;
233235
_status?: ('draft' | 'published') | null;
234236
}
237+
/**
238+
* This interface was referenced by `Config`'s JSON-Schema
239+
* via the `definition` "relationships".
240+
*/
241+
export interface Relationship {
242+
id: number;
243+
tenant?: (number | null) | Tenant;
244+
title: string;
245+
relationship?: (number | null) | Relationship;
246+
updatedAt: string;
247+
createdAt: string;
248+
}
235249
/**
236250
* This interface was referenced by `Config`'s JSON-Schema
237251
* via the `definition` "payload-locked-documents".
238252
*/
239253
export interface PayloadLockedDocument {
240-
id: string;
254+
id: number;
241255
document?:
242256
| ({
243257
relationTo: 'tenants';
244-
value: string | Tenant;
258+
value: number | Tenant;
245259
} | null)
246260
| ({
247261
relationTo: 'users';
248-
value: string | User;
262+
value: number | User;
249263
} | null)
250264
| ({
251265
relationTo: 'food-items';
252-
value: string | FoodItem;
266+
value: number | FoodItem;
253267
} | null)
254268
| ({
255269
relationTo: 'food-menu';
256-
value: string | FoodMenu;
270+
value: number | FoodMenu;
257271
} | null)
258272
| ({
259273
relationTo: 'autosave-global';
260-
value: string | AutosaveGlobal;
274+
value: number | AutosaveGlobal;
275+
} | null)
276+
| ({
277+
relationTo: 'relationships';
278+
value: number | Relationship;
261279
} | null);
262280
globalSlug?: string | null;
263281
user: {
264282
relationTo: 'users';
265-
value: string | User;
283+
value: number | User;
266284
};
267285
updatedAt: string;
268286
createdAt: string;
@@ -272,10 +290,10 @@ export interface PayloadLockedDocument {
272290
* via the `definition` "payload-preferences".
273291
*/
274292
export interface PayloadPreference {
275-
id: string;
293+
id: number;
276294
user: {
277295
relationTo: 'users';
278-
value: string | User;
296+
value: number | User;
279297
};
280298
key?: string | null;
281299
value?:
@@ -295,7 +313,7 @@ export interface PayloadPreference {
295313
* via the `definition` "payload-migrations".
296314
*/
297315
export interface PayloadMigration {
298-
id: string;
316+
id: number;
299317
name?: string | null;
300318
batch?: number | null;
301319
updatedAt: string;
@@ -383,6 +401,17 @@ export interface AutosaveGlobalSelect<T extends boolean = true> {
383401
createdAt?: T;
384402
_status?: T;
385403
}
404+
/**
405+
* This interface was referenced by `Config`'s JSON-Schema
406+
* via the `definition` "relationships_select".
407+
*/
408+
export interface RelationshipsSelect<T extends boolean = true> {
409+
tenant?: T;
410+
title?: T;
411+
relationship?: T;
412+
updatedAt?: T;
413+
createdAt?: T;
414+
}
386415
/**
387416
* This interface was referenced by `Config`'s JSON-Schema
388417
* via the `definition` "payload-locked-documents_select".

0 commit comments

Comments
 (0)