diff --git a/packages/cloud/src/CloudAPI.ts b/packages/cloud/src/CloudAPI.ts index d1c3f89c2b..57f57762a9 100644 --- a/packages/cloud/src/CloudAPI.ts +++ b/packages/cloud/src/CloudAPI.ts @@ -111,6 +111,8 @@ export class CloudAPI { async shareTask(taskId: string, visibility: ShareVisibility = "organization"): Promise { this.log(`[CloudAPI] Sharing task ${taskId} with visibility: ${visibility}`) + // The server should validate that the authenticated user owns this task + // by checking the session token's user ID against the task's owner const response = await this.request("/api/extension/share", { method: "POST", body: JSON.stringify({ taskId, visibility }), diff --git a/packages/cloud/src/bridge/BridgeOrchestrator.ts b/packages/cloud/src/bridge/BridgeOrchestrator.ts index 15b5c65eb2..1d98ee73a1 100644 --- a/packages/cloud/src/bridge/BridgeOrchestrator.ts +++ b/packages/cloud/src/bridge/BridgeOrchestrator.ts @@ -191,6 +191,7 @@ export class BridgeOrchestrator { instanceId: this.instanceId, appProperties: this.appProperties, gitProperties: this.gitProperties, + userId: this.userId, }) } diff --git a/packages/cloud/src/bridge/TaskChannel.ts b/packages/cloud/src/bridge/TaskChannel.ts index 433e740d4e..c20dadd2ac 100644 --- a/packages/cloud/src/bridge/TaskChannel.ts +++ b/packages/cloud/src/bridge/TaskChannel.ts @@ -26,8 +26,9 @@ type TaskEventMapping = { createPayload: (task: TaskLike, ...args: any[]) => any // eslint-disable-line @typescript-eslint/no-explicit-any } -// eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface TaskChannelOptions extends BaseChannelOptions {} +interface TaskChannelOptions extends BaseChannelOptions { + userId?: string +} /** * Manages task-level communication channels. @@ -41,6 +42,7 @@ export class TaskChannel extends BaseChannel< private subscribedTasks: Map = new Map() private pendingTasks: Map = new Map() private taskListeners: Map> = new Map() + private readonly userId?: string private readonly eventMapping: readonly TaskEventMapping[] = [ { @@ -74,6 +76,7 @@ export class TaskChannel extends BaseChannel< constructor(options: TaskChannelOptions) { super(options) + this.userId = options.userId } protected async handleCommandImplementation(command: TaskBridgeCommand): Promise { @@ -160,7 +163,10 @@ export class TaskChannel extends BaseChannel< public async subscribeToTask(task: TaskLike, _socket: Socket): Promise { const taskId = task.taskId - await this.publish(TaskSocketEvents.JOIN, { taskId }, (response: JoinResponse) => { + // Include userId in the join request for server-side validation + const joinPayload = this.userId ? { taskId, userId: this.userId } : { taskId } + + await this.publish(TaskSocketEvents.JOIN, joinPayload, (response: JoinResponse) => { if (response.success) { console.log(`[TaskChannel#subscribeToTask] subscribed to ${taskId}`) this.subscribedTasks.set(taskId, task) diff --git a/packages/cloud/src/bridge/__tests__/TaskChannel.test.ts b/packages/cloud/src/bridge/__tests__/TaskChannel.test.ts index 1f13da9661..c35a1a3d45 100644 --- a/packages/cloud/src/bridge/__tests__/TaskChannel.test.ts +++ b/packages/cloud/src/bridge/__tests__/TaskChannel.test.ts @@ -403,4 +403,130 @@ describe("TaskChannel", () => { errorSpy.mockRestore() }) }) + + describe("User ID Validation", () => { + it("should include userId in JOIN payload when userId is provided", async () => { + const userId = "test-user-123" + const channelWithUserId = new TaskChannel({ + instanceId, + appProperties, + userId, + }) + + // Mock the publish method to capture the payload + let capturedPayload: any = null + const channel = channelWithUserId as any + channel.publish = vi.fn((event: string, data: any, callback?: Function) => { + if (event === TaskSocketEvents.JOIN) { + capturedPayload = data + if (callback) { + callback({ success: true }) + } + } + return true + }) + + await channelWithUserId.onConnect(mockSocket) + await channel.subscribeToTask(mockTask, mockSocket) + + // Verify the JOIN payload includes userId + expect(capturedPayload).toEqual({ + taskId, + userId, + }) + }) + + it("should not include userId in JOIN payload when userId is not provided", async () => { + // Mock the publish method to capture the payload + let capturedPayload: any = null + const channel = taskChannel as any + channel.publish = vi.fn((event: string, data: any, callback?: Function) => { + if (event === TaskSocketEvents.JOIN) { + capturedPayload = data + if (callback) { + callback({ success: true }) + } + } + return true + }) + + await taskChannel.onConnect(mockSocket) + await channel.subscribeToTask(mockTask, mockSocket) + + // Verify the JOIN payload does not include userId + expect(capturedPayload).toEqual({ + taskId, + }) + }) + + it("should handle subscription failure when user is not authorized", async () => { + const userId = "unauthorized-user" + const channelWithUserId = new TaskChannel({ + instanceId, + appProperties, + userId, + }) + + // Mock the publish method to simulate authorization failure + const channel = channelWithUserId as any + channel.publish = vi.fn((event: string, data: any, callback?: Function) => { + if (event === TaskSocketEvents.JOIN && callback) { + // Simulate authorization failure + callback({ + success: false, + error: "User not authorized to access this task", + }) + } + return true + }) + + const errorSpy = vi.spyOn(console, "error") + + await channelWithUserId.onConnect(mockSocket) + await channel.subscribeToTask(mockTask, mockSocket) + + // Verify error was logged + expect(errorSpy).toHaveBeenCalledWith( + `[TaskChannel#subscribeToTask] failed to subscribe to ${taskId}: User not authorized to access this task`, + ) + + // Verify task was not added to subscribedTasks + expect(channel.subscribedTasks.has(taskId)).toBe(false) + + errorSpy.mockRestore() + }) + + it("should successfully subscribe when user is authorized", async () => { + const userId = "authorized-user" + const channelWithUserId = new TaskChannel({ + instanceId, + appProperties, + userId, + }) + + // Mock the publish method to simulate successful authorization + const channel = channelWithUserId as any + channel.publish = vi.fn((event: string, data: any, callback?: Function) => { + if (event === TaskSocketEvents.JOIN && callback) { + // Simulate successful authorization + callback({ success: true }) + } + return true + }) + + const logSpy = vi.spyOn(console, "log") + + await channelWithUserId.onConnect(mockSocket) + await channel.subscribeToTask(mockTask, mockSocket) + + // Verify success was logged + expect(logSpy).toHaveBeenCalledWith(`[TaskChannel#subscribeToTask] subscribed to ${taskId}`) + + // Verify task was added to subscribedTasks + expect(channel.subscribedTasks.has(taskId)).toBe(true) + expect(channel.subscribedTasks.get(taskId)).toBe(mockTask) + + logSpy.mockRestore() + }) + }) })