fix(server): scope personal favorite SSE events to their owner (#23712)
## Problem Fixes #20483. In a multi-user workspace, when a user creates/updates/deletes a **personal favorite** (a `navigationMenuItem` with a non-null `userWorkspaceId`), the metadata SSE event is broadcast workspace-wide. Every other connected user receives it and the favorite pops into their own sidebar in real time. Cross-user data-isolation leak. ## Root cause The delivery filter in `WorkspaceEventBroadcaster` already supports per-user scoping via `recipientUserWorkspaceIds`, but treats an **undefined** list as workspace-wide (delivered to every stream). `MetadataEventPublisher.publish` never set that field, so every favorite event fell into the workspace-wide default. The `agentChatThread` path already sets `recipientUserWorkspaceIds` explicitly and is scoped correctly; favorites simply never opted in. ## Fix In `MetadataEventPublisher`, resolve the owning `userWorkspaceId` for `navigationMenuItem` events (from `properties.after` on create/update, `properties.before` on delete) and set `recipientUserWorkspaceIds: [userWorkspaceId]` when present. Workspace-level items (`userWorkspaceId === null`) leave it unset and keep broadcasting to everyone. This mirrors the existing `agentChatThread` precedent and touches only the producer, not the broadcaster or any consumer. ## Testing Unit test (`metadata-event-publisher.spec.ts`) covers personal create/update/delete (scoped to owner), workspace-level (unscoped), and an unrelated metadata entity carrying a user id (unscoped). Also verified end to end against a local multi-user workspace (Tim and Jane, same workspace): each opened a live SSE stream (`/metadata` `onEventSubscription`) and Tim created favorites. | Case | Before fix | After fix | |------|-----------|-----------| | Personal favorite -> owner (Tim) | receives | receives | | Personal favorite -> other user (Jane) | **receives (leak)** | not received | | Workspace-level favorite -> other user (Jane) | receives | receives | - `nx typecheck twenty-server`: pass - oxlint + oxfmt on changed files: clean ## Scope / follow-up Favorites only. Two related items are intentionally out of scope and worth tracking separately: scoping other user-owned metadata (`view` via its visibility rules, `roleTarget`), and making the broadcaster's "no recipient list = everyone" default explicit rather than fail-open.
This commit is contained in:
Thomas Trompette
+172
@@ -0,0 +1,172 @@
|
||||
import { Test, type TestingModule } from '@nestjs/testing';
|
||||
|
||||
import { I18nService } from 'src/engine/core-modules/i18n/i18n.service';
|
||||
import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service';
|
||||
import { NavigationMenuItemRecordIdentifierService } from 'src/engine/metadata-modules/navigation-menu-item/services/navigation-menu-item-record-identifier.service';
|
||||
import { MetadataEventPublisher } from 'src/engine/subscriptions/metadata-event/metadata-event-publisher';
|
||||
import { type MetadataEventBatch } from 'src/engine/subscriptions/metadata-event/types/metadata-event-batch.type';
|
||||
import { WorkspaceEventBroadcaster } from 'src/engine/subscriptions/workspace-event-broadcaster/workspace-event-broadcaster.service';
|
||||
|
||||
const OWNER_USER_WORKSPACE_ID = '20202020-0000-0000-0000-000000000001';
|
||||
|
||||
describe('MetadataEventPublisher', () => {
|
||||
let publisher: MetadataEventPublisher;
|
||||
const broadcast = jest.fn();
|
||||
|
||||
beforeEach(async () => {
|
||||
jest.clearAllMocks();
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
MetadataEventPublisher,
|
||||
{ provide: WorkspaceEventBroadcaster, useValue: { broadcast } },
|
||||
{
|
||||
provide: WorkspaceManyOrAllFlatEntityMapsCacheService,
|
||||
useValue: {},
|
||||
},
|
||||
{ provide: NavigationMenuItemRecordIdentifierService, useValue: {} },
|
||||
{ provide: I18nService, useValue: {} },
|
||||
],
|
||||
}).compile();
|
||||
|
||||
publisher = module.get<MetadataEventPublisher>(MetadataEventPublisher);
|
||||
});
|
||||
|
||||
const publishAndGetFirstEvent = async (batch: object) => {
|
||||
await publisher.publish(batch as unknown as MetadataEventBatch);
|
||||
|
||||
return broadcast.mock.calls[0][0].events[0];
|
||||
};
|
||||
|
||||
it('scopes a personal favorite create event to its owner', async () => {
|
||||
const event = await publishAndGetFirstEvent({
|
||||
name: 'metadata.navigationMenuItem.created',
|
||||
workspaceId: 'workspace-1',
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'created',
|
||||
events: [
|
||||
{
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'created',
|
||||
recordId: 'nav-1',
|
||||
properties: {
|
||||
after: { id: 'nav-1', userWorkspaceId: OWNER_USER_WORKSPACE_ID },
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(event.recipientUserWorkspaceIds).toEqual([OWNER_USER_WORKSPACE_ID]);
|
||||
});
|
||||
|
||||
it('scopes a personal favorite update event to its owner', async () => {
|
||||
const event = await publishAndGetFirstEvent({
|
||||
name: 'metadata.navigationMenuItem.updated',
|
||||
workspaceId: 'workspace-1',
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'updated',
|
||||
events: [
|
||||
{
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'updated',
|
||||
recordId: 'nav-1',
|
||||
properties: {
|
||||
updatedFields: ['name'],
|
||||
diff: {},
|
||||
before: { id: 'nav-1', userWorkspaceId: OWNER_USER_WORKSPACE_ID },
|
||||
after: { id: 'nav-1', userWorkspaceId: OWNER_USER_WORKSPACE_ID },
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(event.recipientUserWorkspaceIds).toEqual([OWNER_USER_WORKSPACE_ID]);
|
||||
});
|
||||
|
||||
it('scopes a personal favorite delete event to its owner using the pre-delete record', async () => {
|
||||
const event = await publishAndGetFirstEvent({
|
||||
name: 'metadata.navigationMenuItem.deleted',
|
||||
workspaceId: 'workspace-1',
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'deleted',
|
||||
events: [
|
||||
{
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'deleted',
|
||||
recordId: 'nav-1',
|
||||
properties: {
|
||||
before: { id: 'nav-1', userWorkspaceId: OWNER_USER_WORKSPACE_ID },
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(event.recipientUserWorkspaceIds).toEqual([OWNER_USER_WORKSPACE_ID]);
|
||||
});
|
||||
|
||||
it('broadcasts a workspace-level favorite (null owner) to everyone', async () => {
|
||||
const event = await publishAndGetFirstEvent({
|
||||
name: 'metadata.navigationMenuItem.created',
|
||||
workspaceId: 'workspace-1',
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'created',
|
||||
events: [
|
||||
{
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'created',
|
||||
recordId: 'nav-1',
|
||||
properties: {
|
||||
after: { id: 'nav-1', userWorkspaceId: null },
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(event.recipientUserWorkspaceIds).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not scope a favorite with an empty-string owner', async () => {
|
||||
const event = await publishAndGetFirstEvent({
|
||||
name: 'metadata.navigationMenuItem.created',
|
||||
workspaceId: 'workspace-1',
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'created',
|
||||
events: [
|
||||
{
|
||||
metadataName: 'navigationMenuItem',
|
||||
type: 'created',
|
||||
recordId: 'nav-1',
|
||||
properties: {
|
||||
after: { id: 'nav-1', userWorkspaceId: '' },
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(event.recipientUserWorkspaceIds).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not scope unrelated metadata even when it carries a userWorkspaceId', async () => {
|
||||
const event = await publishAndGetFirstEvent({
|
||||
name: 'metadata.view.created',
|
||||
workspaceId: 'workspace-1',
|
||||
metadataName: 'view',
|
||||
type: 'created',
|
||||
events: [
|
||||
{
|
||||
metadataName: 'view',
|
||||
type: 'created',
|
||||
recordId: 'view-1',
|
||||
properties: {
|
||||
after: {
|
||||
id: 'view-1',
|
||||
createdByUserWorkspaceId: OWNER_USER_WORKSPACE_ID,
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(event.recipientUserWorkspaceIds).toBeUndefined();
|
||||
});
|
||||
});
|
||||
+30
-6
@@ -1,5 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
import { isNonEmptyString } from '@sniptt/guards';
|
||||
import { SOURCE_LOCALE } from 'twenty-shared/translations';
|
||||
import { isDefined, isNonEmptyArray } from 'twenty-shared/utils';
|
||||
|
||||
@@ -34,15 +35,38 @@ export class MetadataEventPublisher {
|
||||
await this.workspaceEventBroadcaster.broadcast({
|
||||
workspaceId: enrichedBatch.workspaceId,
|
||||
updatedCollectionHash: enrichedBatch.updatedCollectionHash,
|
||||
events: enrichedBatch.events.map((event) => ({
|
||||
type: event.type,
|
||||
entityName: event.metadataName,
|
||||
recordId: event.recordId,
|
||||
properties: event.properties as Record<string, unknown>,
|
||||
})),
|
||||
events: enrichedBatch.events.map((event) => {
|
||||
const ownerUserWorkspaceId = this.resolveOwnerUserWorkspaceId(event);
|
||||
|
||||
return {
|
||||
type: event.type,
|
||||
entityName: event.metadataName,
|
||||
recordId: event.recordId,
|
||||
properties: event.properties as Record<string, unknown>,
|
||||
recipientUserWorkspaceIds: isNonEmptyString(ownerUserWorkspaceId)
|
||||
? [ownerUserWorkspaceId]
|
||||
: undefined,
|
||||
};
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
private resolveOwnerUserWorkspaceId(
|
||||
event: MetadataEventBatch['events'][number],
|
||||
): string | undefined {
|
||||
if (event.metadataName !== 'navigationMenuItem') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const record = (
|
||||
event.type === 'deleted'
|
||||
? event.properties.before
|
||||
: event.properties.after
|
||||
) as { userWorkspaceId?: string | null } | undefined;
|
||||
|
||||
return record?.userWorkspaceId ?? undefined;
|
||||
}
|
||||
|
||||
private async enrichMetadataEventBatch(
|
||||
metadataEventBatch: MetadataEventBatch,
|
||||
): Promise<MetadataEventBatch> {
|
||||
|
||||
Reference in New Issue
Block a user