From 1e265eaba3982cd48bad50b05b0bee90ef889415 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 8 Jul 2025 00:54:10 -0400 Subject: [PATCH] Replace skipAuth with shareToken --- apps/web/src/actions/analytics/events.ts | 125 ++++++++++++++++-- apps/web/src/actions/analytics/messages.ts | 78 ++++++++++- apps/web/src/actions/taskSharing.ts | 18 +-- .../app/(authenticated)/usage/TaskModal.tsx | 2 +- 4 files changed, 197 insertions(+), 26 deletions(-) diff --git a/apps/web/src/actions/analytics/events.ts b/apps/web/src/actions/analytics/events.ts index bfe44141ca..795b0b8fc4 100644 --- a/apps/web/src/actions/analytics/events.ts +++ b/apps/web/src/actions/analytics/events.ts @@ -12,8 +12,17 @@ import type { Filter } from '@/types/analytics'; import { buildFilterConditions } from '@/types/analytics'; import { analytics } from '@/lib/server'; import { tokenSumSql } from '@/lib'; -import { type User, getUsersById } from '@roo-code-cloud/db/server'; +import { + type User, + getUsersById, + db, + taskShares, +} from '@roo-code-cloud/db/server'; import { authorizeAnalytics } from '@/actions/auth'; +import { eq } from 'drizzle-orm'; +import { isValidShareToken, isShareExpired } from '@/lib/task-sharing'; +import { TaskShareVisibility } from '@/types'; +import { auth } from '@clerk/nextjs/server'; type Table = 'events' | 'messages'; @@ -64,6 +73,68 @@ export const captureEvent = async ({ event, ...rest }: AnalyticsEvent) => { await analytics.insert({ table, values: [value], format: 'JSONEachRow' }); }; +/** + * SECURITY: Share token authorization for public access + * + * This function validates share tokens and returns scoped access permissions + * for public task sharing without bypassing all security controls. + * + * @param shareToken - The share token to validate + * @returns Authorization result with scoped access or null if invalid + */ +async function authorizeShareToken(shareToken: string): Promise<{ + isValid: boolean; + taskId?: string; + orgId?: string | null; + userId?: string; + visibility?: string; +} | null> { + try { + if (!isValidShareToken(shareToken)) { + return null; + } + + // Get the share from database + const [shareWithUser] = await db + .select({ + share: taskShares, + }) + .from(taskShares) + .where(eq(taskShares.shareToken, shareToken)) + .limit(1); + + if (!shareWithUser) { + return null; + } + + const { share } = shareWithUser; + + if (isShareExpired(share.expiresAt)) { + return null; + } + + // For organization shares, verify the user has access to the org + if (share.visibility === TaskShareVisibility.ORGANIZATION) { + const { userId, orgId } = await auth(); + + if (!userId || !orgId || orgId !== share.orgId) { + return null; // Organization shares require matching org membership + } + } + + return { + isValid: true, + taskId: share.taskId, + orgId: share.orgId, + userId: share.createdByUserId, + visibility: share.visibility, + }; + } catch (error) { + console.error('Error validating share token:', error); + return null; + } +} + /** * SECURITY: Standardized access control filter builder * @@ -524,7 +595,7 @@ export const getTasks = async ({ orgId, userId, taskId, - skipAuth = false, + shareToken, limit = 20, cursor, filters = [], @@ -532,15 +603,33 @@ export const getTasks = async ({ orgId?: string | null; userId?: string | null; taskId?: string | null; - skipAuth?: boolean; + shareToken?: string; limit?: number; cursor?: number; filters?: Filter[]; }): Promise => { let authUserId: string | null = null; let isAdmin = false; + let isShareAccess = false; - if (!skipAuth) { + // Handle share token authorization (for public shares) + if (shareToken) { + const shareAuth = await authorizeShareToken(shareToken); + if (!shareAuth?.isValid) { + return { tasks: [], hasMore: false }; // Invalid share token + } + + // For share access, we only allow querying the specific shared task + if (taskId && taskId !== shareAuth.taskId) { + return { tasks: [], hasMore: false }; // Share token doesn't match requested task + } + + // Override parameters with share-scoped values + taskId = shareAuth.taskId; + orgId = shareAuth.orgId; + isShareAccess = true; + } else { + // Normal authentication flow const authResult = await authorizeAnalytics({ requestedOrgId: orgId, requestedUserId: userId, @@ -550,9 +639,9 @@ export const getTasks = async ({ } // For personal accounts, query by userId instead of orgId - // Exception: when skipAuth is true (for public shares), we can query without userId - if (!orgId && !authUserId && !skipAuth) { - return { tasks: [], hasMore: false }; // Personal accounts must have a userId unless we're skipping auth + // Exception: when using share token, access is already validated + if (!orgId && !authUserId && !isShareAccess) { + return { tasks: [], hasMore: false }; // Personal accounts must have a userId unless using share access } const taskFilter = taskId ? 'AND e.taskId = {taskId: String}' : ''; @@ -563,7 +652,8 @@ export const getTasks = async ({ let messagesAccessFilter = 'WHERE 1=1'; let accessParams: Record = {}; - if (!skipAuth) { + if (!isShareAccess) { + // Normal access control for authenticated users const { accessFilter, accessParams: params } = buildAccessControlFilter( authUserId, isAdmin, @@ -580,9 +670,21 @@ export const getTasks = async ({ orgId, ); messagesAccessFilter = `WHERE ${messageFilter}`; + } else { + // Share access: only allow access to the specific shared task + if (taskId) { + eventsAccessFilter = 'WHERE e.taskId = {taskId: String}'; + messagesAccessFilter = 'WHERE taskId = {taskId: String}'; + if (orgId) { + eventsAccessFilter += ' AND e.orgId = {orgId: String}'; + messagesAccessFilter += ' AND orgId = {orgId: String}'; + accessParams.orgId = orgId; + } else { + eventsAccessFilter += ' AND e.orgId IS NULL'; + messagesAccessFilter += ' AND orgId IS NULL'; + } + } } - // When skipAuth=true (for public shares), use 'WHERE 1=1' to ensure valid SQL - // The share token itself provides the authorization const queryParams: Record = { types: [ @@ -775,15 +877,18 @@ export const getTaskById = async ({ taskId, orgId, userId, + shareToken, }: { taskId: string; orgId?: string | null; userId?: string | null; + shareToken?: string; }): Promise => { const result = await getTasks({ taskId, orgId, userId, + shareToken, limit: 1, }); diff --git a/apps/web/src/actions/analytics/messages.ts b/apps/web/src/actions/analytics/messages.ts index b2513e20fe..1cb0de62c1 100644 --- a/apps/web/src/actions/analytics/messages.ts +++ b/apps/web/src/actions/analytics/messages.ts @@ -4,6 +4,11 @@ import { z } from 'zod'; import { analytics } from '@/lib/server'; import { authorizeAnalytics } from '@/actions/auth'; +import { db, taskShares } from '@roo-code-cloud/db/server'; +import { eq } from 'drizzle-orm'; +import { isValidShareToken, isShareExpired } from '@/lib/task-sharing'; +import { TaskShareVisibility } from '@/types'; +import { auth } from '@clerk/nextjs/server'; /** * getMessages @@ -27,15 +32,80 @@ const messageSchema = z.object({ export type Message = z.infer; +/** + * SECURITY: Share token authorization for messages + */ +async function authorizeMessageShareToken(shareToken: string): Promise<{ + isValid: boolean; + taskId?: string; + orgId?: string | null; +} | null> { + try { + if (!isValidShareToken(shareToken)) { + return null; + } + + const [shareWithUser] = await db + .select({ + share: taskShares, + }) + .from(taskShares) + .where(eq(taskShares.shareToken, shareToken)) + .limit(1); + + if (!shareWithUser) { + return null; + } + + const { share } = shareWithUser; + + if (isShareExpired(share.expiresAt)) { + return null; + } + + // For organization shares, verify the user has access to the org + if (share.visibility === TaskShareVisibility.ORGANIZATION) { + const { userId, orgId } = await auth(); + + if (!userId || !orgId || orgId !== share.orgId) { + return null; + } + } + + return { + isValid: true, + taskId: share.taskId, + orgId: share.orgId, + }; + } catch (error) { + console.error('Error validating message share token:', error); + return null; + } +} + export const getMessages = async ( taskId: string, orgId?: string | null, userId?: string | null, - skipAuth = false, + shareToken?: string, ): Promise => { - // Authorize the request - this will handle both personal and org contexts - // Skip auth for public shares viewed by unauthenticated users - if (!skipAuth) { + // Handle share token authorization (for public shares) + if (shareToken) { + const shareAuth = await authorizeMessageShareToken(shareToken); + if (!shareAuth?.isValid) { + return []; // Invalid share token + } + + // For share access, we only allow querying the specific shared task + if (taskId !== shareAuth.taskId) { + return []; // Share token doesn't match requested task + } + + // Override parameters with share-scoped values + taskId = shareAuth.taskId; + orgId = shareAuth.orgId; + } else { + // Normal authentication flow await authorizeAnalytics({ requestedOrgId: orgId, requestedUserId: userId, diff --git a/apps/web/src/actions/taskSharing.ts b/apps/web/src/actions/taskSharing.ts index c16aad2538..eb379f28c6 100644 --- a/apps/web/src/actions/taskSharing.ts +++ b/apps/web/src/actions/taskSharing.ts @@ -291,13 +291,10 @@ export async function getTaskByShareToken(token: string): Promise<{ } // For public shares (including personal shares), no auth check needed - // Get task data based on visibility - // For organization shares, we need to skip auth since the viewer might be a different user - // but they're authorized to view this share within their organization + // Get task data using secure share token authorization const result = await getTasks({ taskId: share.taskId, - orgId: share.orgId, // Will be null for personal shares - skipAuth: true, // Skip auth for both public and organization shares since we've already validated access above + shareToken: token, // Use share token for secure scoped access }); const task = result.tasks[0]; @@ -309,7 +306,7 @@ export async function getTaskByShareToken(token: string): Promise<{ share.taskId, share.orgId, task.userId, - true, // Skip auth for both public and organization shares since we've already validated access above + token, // Use share token for secure scoped access ); return { @@ -525,11 +522,10 @@ export async function getSharedTaskMessages( } // For public shares, no auth check needed - // Get the task to get the userId + // Get the task using secure share token authorization const result = await getTasks({ taskId: share.taskId, - orgId: share.orgId, - skipAuth: true, // We've already validated access above + shareToken: shareToken, // Use share token for secure scoped access }); const task = result.tasks[0]; @@ -537,12 +533,12 @@ export async function getSharedTaskMessages( throw new Error('Task not found'); } - // Get messages with skipAuth since we've already validated access + // Get messages using secure share token authorization const messages = await getMessages( share.taskId, share.orgId, task.userId, - true, // Skip auth since we've already validated access above + shareToken, // Use share token for secure scoped access ); return messages; diff --git a/apps/web/src/app/(authenticated)/usage/TaskModal.tsx b/apps/web/src/app/(authenticated)/usage/TaskModal.tsx index cfcb42f735..4ed0db8d29 100644 --- a/apps/web/src/app/(authenticated)/usage/TaskModal.tsx +++ b/apps/web/src/app/(authenticated)/usage/TaskModal.tsx @@ -25,7 +25,7 @@ export const TaskModal = ({ task, open, onClose }: TaskModalProps) => { const { data: messages = [] } = useQuery({ queryKey: ['messages', task.taskId, orgId, userId], - queryFn: () => getMessages(task.taskId, orgId, userId, false), + queryFn: () => getMessages(task.taskId, orgId, userId), enabled: open && !!task.taskId, ...messagePolling, });