mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
Admins should be able to share all tasks (#76)
* Admins should be able to share all tasks * PR feedback
This commit is contained in:
parent
2d6091f5e7
commit
8101f2026d
6 changed files with 290 additions and 45 deletions
125
src/actions/__tests__/taskSharing.test.ts
Normal file
125
src/actions/__tests__/taskSharing.test.ts
Normal file
|
|
@ -0,0 +1,125 @@
|
|||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import type { TaskWithUser } from '../analytics';
|
||||
|
||||
// Create a mock function for the canShareTask function
|
||||
const mockCanShareTask = vi.fn();
|
||||
|
||||
// Mock the taskSharing module
|
||||
vi.mock('../taskSharing', async () => {
|
||||
const actual = await vi.importActual('../taskSharing');
|
||||
return {
|
||||
...actual,
|
||||
canShareTask: mockCanShareTask,
|
||||
};
|
||||
});
|
||||
|
||||
// Mock the analytics module
|
||||
vi.mock('../analytics', () => ({
|
||||
getTasks: vi.fn(),
|
||||
}));
|
||||
|
||||
describe('Task Sharing Permissions', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should allow admin to share any task', async () => {
|
||||
const mockTask: TaskWithUser = {
|
||||
taskId: 'task-123',
|
||||
userId: 'other-user',
|
||||
user: {
|
||||
id: 'other-user',
|
||||
name: 'Other User',
|
||||
email: 'other@example.com',
|
||||
imageUrl: 'https://example.com/avatar.jpg',
|
||||
entity: {},
|
||||
lastSyncAt: new Date(),
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
orgId: 'org-123',
|
||||
orgRole: 'org:member',
|
||||
},
|
||||
title: 'Test Task',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
mode: 'code',
|
||||
completed: false,
|
||||
tokens: 1000,
|
||||
cost: 0.02,
|
||||
timestamp: 1234567890,
|
||||
};
|
||||
|
||||
mockCanShareTask.mockResolvedValue({
|
||||
canShare: true,
|
||||
task: mockTask,
|
||||
});
|
||||
|
||||
const result = await mockCanShareTask('task-123');
|
||||
|
||||
expect(result.canShare).toBe(true);
|
||||
expect(result.task).toEqual(mockTask);
|
||||
});
|
||||
|
||||
it('should allow member to share their own task', async () => {
|
||||
const mockTask: TaskWithUser = {
|
||||
taskId: 'task-123',
|
||||
userId: 'member-user',
|
||||
user: {
|
||||
id: 'member-user',
|
||||
name: 'Member User',
|
||||
email: 'member@example.com',
|
||||
imageUrl: 'https://example.com/avatar.jpg',
|
||||
entity: {},
|
||||
lastSyncAt: new Date(),
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
orgId: 'org-123',
|
||||
orgRole: 'org:member',
|
||||
},
|
||||
title: 'Test Task',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
mode: 'code',
|
||||
completed: false,
|
||||
tokens: 1000,
|
||||
cost: 0.02,
|
||||
timestamp: 1234567890,
|
||||
};
|
||||
|
||||
mockCanShareTask.mockResolvedValue({
|
||||
canShare: true,
|
||||
task: mockTask,
|
||||
});
|
||||
|
||||
const result = await mockCanShareTask('task-123');
|
||||
|
||||
expect(result.canShare).toBe(true);
|
||||
expect(result.task).toEqual(mockTask);
|
||||
});
|
||||
|
||||
it('should not allow member to share other users task', async () => {
|
||||
mockCanShareTask.mockResolvedValue({
|
||||
canShare: false,
|
||||
error: 'Task not found or you do not have permission to share this task',
|
||||
});
|
||||
|
||||
const result = await mockCanShareTask('task-123');
|
||||
|
||||
expect(result.canShare).toBe(false);
|
||||
expect(result.error).toBe(
|
||||
'Task not found or you do not have permission to share this task',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle task not found for admin', async () => {
|
||||
mockCanShareTask.mockResolvedValue({
|
||||
canShare: false,
|
||||
error: 'Task not found',
|
||||
});
|
||||
|
||||
const result = await mockCanShareTask('nonexistent-task');
|
||||
|
||||
expect(result.canShare).toBe(false);
|
||||
expect(result.error).toBe('Task not found');
|
||||
});
|
||||
});
|
||||
|
|
@ -22,10 +22,12 @@ async function validateAnalyticsAccess({
|
|||
requestedOrgId,
|
||||
requestedUserId,
|
||||
requireAdmin = false,
|
||||
allowCrossUserAccess = false,
|
||||
}: {
|
||||
requestedOrgId?: string | null;
|
||||
requestedUserId?: string | null;
|
||||
requireAdmin?: boolean;
|
||||
allowCrossUserAccess?: boolean;
|
||||
}): Promise<{
|
||||
authOrgId: string;
|
||||
authUserId: string;
|
||||
|
|
@ -54,8 +56,11 @@ async function validateAnalyticsAccess({
|
|||
}
|
||||
|
||||
// For non-admin users, force userId filter to their own ID
|
||||
// Unless allowCrossUserAccess is true and we're checking task sharing permissions
|
||||
const effectiveUserId =
|
||||
orgRole !== 'org:admin' ? authUserId : requestedUserId || null;
|
||||
orgRole !== 'org:admin' && !allowCrossUserAccess
|
||||
? authUserId
|
||||
: requestedUserId || null;
|
||||
|
||||
return {
|
||||
authOrgId,
|
||||
|
|
@ -347,13 +352,18 @@ export type TaskWithUser = TaskWithTitle & { user: User };
|
|||
export const getTasks = async ({
|
||||
orgId,
|
||||
userId,
|
||||
taskId,
|
||||
allowCrossUserAccess = false,
|
||||
}: {
|
||||
orgId?: string | null;
|
||||
userId?: string | null;
|
||||
taskId?: string | null;
|
||||
allowCrossUserAccess?: boolean;
|
||||
}): Promise<TaskWithUser[]> => {
|
||||
const { effectiveUserId } = await validateAnalyticsAccess({
|
||||
requestedOrgId: orgId,
|
||||
requestedUserId: userId,
|
||||
allowCrossUserAccess,
|
||||
});
|
||||
|
||||
if (!orgId) {
|
||||
|
|
@ -361,9 +371,12 @@ export const getTasks = async ({
|
|||
}
|
||||
|
||||
const userFilter = effectiveUserId ? 'AND e.userId = {userId: String}' : '';
|
||||
const taskFilter = taskId ? 'AND e.taskId = {taskId: String}' : '';
|
||||
const messageUserFilter = effectiveUserId
|
||||
? 'AND userId = {userId: String}'
|
||||
: '';
|
||||
const messageTaskFilter = taskId ? 'AND taskId = {taskId: String}' : '';
|
||||
|
||||
const queryParams: Record<string, string | string[]> = {
|
||||
orgId: orgId!,
|
||||
types: [
|
||||
|
|
@ -375,6 +388,9 @@ export const getTasks = async ({
|
|||
if (effectiveUserId) {
|
||||
queryParams.userId = effectiveUserId;
|
||||
}
|
||||
if (taskId) {
|
||||
queryParams.taskId = taskId;
|
||||
}
|
||||
|
||||
const results = await analytics.query({
|
||||
query: `
|
||||
|
|
@ -386,6 +402,7 @@ export const getTasks = async ({
|
|||
FROM messages
|
||||
WHERE orgId = {orgId: String}
|
||||
${messageUserFilter}
|
||||
${messageTaskFilter}
|
||||
GROUP BY taskId
|
||||
)
|
||||
SELECT
|
||||
|
|
@ -405,6 +422,7 @@ export const getTasks = async ({
|
|||
e.orgId = {orgId: String}
|
||||
AND e.type IN ({types: Array(String)})
|
||||
${userFilter}
|
||||
${taskFilter}
|
||||
GROUP BY 1, 2
|
||||
ORDER BY timestamp DESC
|
||||
`,
|
||||
|
|
|
|||
|
|
@ -6,9 +6,9 @@ import type { ApiResponse } from '@/types';
|
|||
import { Env, logger } from '@/lib/server';
|
||||
|
||||
export async function validateAuth(): Promise<
|
||||
{ userId: string; orgId: string } | ApiResponse
|
||||
{ userId: string; orgId: string; orgRole: string } | ApiResponse
|
||||
> {
|
||||
const { userId, orgId } = await auth();
|
||||
const { userId, orgId, orgRole } = await auth();
|
||||
|
||||
if (!userId) {
|
||||
return { success: false, error: 'Unauthorized: User required' };
|
||||
|
|
@ -18,7 +18,7 @@ export async function validateAuth(): Promise<
|
|||
return { success: false, error: 'Unauthorized: Organization required' };
|
||||
}
|
||||
|
||||
return { userId, orgId };
|
||||
return { userId, orgId, orgRole: orgRole || 'unknown' };
|
||||
}
|
||||
|
||||
// Default expiration is 30 days (2592000 seconds).
|
||||
|
|
|
|||
|
|
@ -34,18 +34,72 @@ type TaskShareResponse = ApiResponse & {
|
|||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* Check if the current user can share a specific task (for UI components)
|
||||
*/
|
||||
export async function canShareTask(taskId: string): Promise<{
|
||||
canShare: boolean;
|
||||
task?: TaskWithUser;
|
||||
error?: string;
|
||||
userId?: string;
|
||||
orgId?: string;
|
||||
orgRole?: string;
|
||||
}> {
|
||||
try {
|
||||
// Get authentication info
|
||||
const { userId, orgId, orgRole } = await auth();
|
||||
|
||||
if (!userId || !orgId) {
|
||||
return { canShare: false, error: 'Authentication required' };
|
||||
}
|
||||
|
||||
// Admins can share any task in the organization
|
||||
if (orgRole === 'org:admin') {
|
||||
const tasks = await getTasks({
|
||||
taskId,
|
||||
orgId,
|
||||
allowCrossUserAccess: true,
|
||||
});
|
||||
const task = tasks[0];
|
||||
|
||||
if (!task) {
|
||||
return { canShare: false, error: 'Task not found' };
|
||||
}
|
||||
|
||||
return { canShare: true, task, userId, orgId, orgRole };
|
||||
}
|
||||
|
||||
// Members can only share tasks they created
|
||||
const tasks = await getTasks({ taskId, orgId });
|
||||
const task = tasks[0];
|
||||
|
||||
// Additional check: ensure the task belongs to the requesting user
|
||||
if (task && task.userId !== userId) {
|
||||
return {
|
||||
canShare: false,
|
||||
error:
|
||||
'Task not found or you do not have permission to share this task',
|
||||
};
|
||||
}
|
||||
|
||||
if (!task) {
|
||||
return {
|
||||
canShare: false,
|
||||
error:
|
||||
'Task not found or you do not have permission to share this task',
|
||||
};
|
||||
}
|
||||
|
||||
return { canShare: true, task, userId, orgId, orgRole };
|
||||
} catch (_error) {
|
||||
return { canShare: false, error: 'Failed to verify task access' };
|
||||
}
|
||||
}
|
||||
|
||||
export async function createTaskShare(
|
||||
data: CreateTaskShareRequest,
|
||||
): Promise<TaskShareResponse> {
|
||||
try {
|
||||
const authResult = await validateAuth();
|
||||
|
||||
if (!isAuthSuccess(authResult)) {
|
||||
return authResult;
|
||||
}
|
||||
|
||||
const { userId, orgId } = authResult;
|
||||
|
||||
const result = createTaskShareSchema.safeParse(data);
|
||||
|
||||
if (!result.success) {
|
||||
|
|
@ -63,11 +117,19 @@ export async function createTaskShare(
|
|||
};
|
||||
}
|
||||
|
||||
const tasks = await getTasks({ orgId, userId });
|
||||
const task = tasks.find((t) => t.taskId === taskId);
|
||||
// Check if user can share this specific task (includes auth)
|
||||
const { canShare, task, error, userId, orgId, orgRole } =
|
||||
await canShareTask(taskId);
|
||||
|
||||
if (!task) {
|
||||
return { success: false, error: 'Task not found or access denied' };
|
||||
if (!canShare) {
|
||||
return { success: false, error: error || 'Access denied' };
|
||||
}
|
||||
|
||||
if (!task || !userId || !orgId || !orgRole) {
|
||||
return {
|
||||
success: false,
|
||||
error: 'Task not found or authentication failed',
|
||||
};
|
||||
}
|
||||
|
||||
const expirationDaysToUse =
|
||||
|
|
@ -103,8 +165,14 @@ export async function createTaskShare(
|
|||
action: 'created',
|
||||
shareId: insertedShare[0].id,
|
||||
expiresAt: expiresAt.toISOString(),
|
||||
taskOwnerId: task.userId,
|
||||
sharedByAdmin: orgRole === 'org:admin' && task.userId !== userId,
|
||||
},
|
||||
description: `Created task share for task ${taskId}`,
|
||||
description: `Created task share for task ${taskId}${
|
||||
orgRole === 'org:admin' && task.userId !== userId
|
||||
? ` (admin sharing task created by ${task.user.name})`
|
||||
: ''
|
||||
}`,
|
||||
});
|
||||
|
||||
return insertedShare;
|
||||
|
|
@ -159,8 +227,12 @@ export async function getTaskByShareToken(
|
|||
return null;
|
||||
}
|
||||
|
||||
const tasks = await getTasks({ orgId: share.orgId });
|
||||
const task = tasks.find((t) => t.taskId === share.taskId);
|
||||
const tasks = await getTasks({
|
||||
taskId: share.taskId,
|
||||
orgId: share.orgId,
|
||||
allowCrossUserAccess: true,
|
||||
});
|
||||
const task = tasks[0];
|
||||
|
||||
if (!task) {
|
||||
return null;
|
||||
|
|
@ -190,7 +262,7 @@ export async function deleteTaskShare(shareId: string): Promise<ApiResponse> {
|
|||
return authResult;
|
||||
}
|
||||
|
||||
const { userId, orgId } = authResult;
|
||||
const { userId, orgId, orgRole } = authResult;
|
||||
|
||||
const shareIdResult = shareIdSchema.safeParse(shareId);
|
||||
|
||||
|
|
@ -198,20 +270,24 @@ export async function deleteTaskShare(shareId: string): Promise<ApiResponse> {
|
|||
return { success: false, error: 'Invalid share ID format' };
|
||||
}
|
||||
|
||||
// First, find the share to check permissions
|
||||
const [share] = await db
|
||||
.select()
|
||||
.from(taskShares)
|
||||
.where(
|
||||
and(
|
||||
eq(taskShares.id, shareId),
|
||||
eq(taskShares.orgId, orgId),
|
||||
eq(taskShares.createdByUserId, userId),
|
||||
),
|
||||
)
|
||||
.where(and(eq(taskShares.id, shareId), eq(taskShares.orgId, orgId)))
|
||||
.limit(1);
|
||||
|
||||
if (!share) {
|
||||
return { success: false, error: 'Share not found or access denied' };
|
||||
return { success: false, error: 'Share not found' };
|
||||
}
|
||||
|
||||
// Check if user can delete this share
|
||||
// Admins can delete any share, members can only delete shares they created
|
||||
if (orgRole !== 'org:admin' && share.createdByUserId !== userId) {
|
||||
return {
|
||||
success: false,
|
||||
error: 'Access denied: You can only delete shares you created',
|
||||
};
|
||||
}
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
|
|
@ -222,8 +298,17 @@ export async function deleteTaskShare(shareId: string): Promise<ApiResponse> {
|
|||
orgId,
|
||||
targetType: AuditLogTargetType.TASK_SHARE,
|
||||
targetId: share.taskId,
|
||||
newValue: { action: 'deleted', shareId: share.id },
|
||||
description: `Deleted task share for task ${share.taskId}`,
|
||||
newValue: {
|
||||
action: 'deleted',
|
||||
shareId: share.id,
|
||||
deletedByAdmin:
|
||||
orgRole === 'org:admin' && share.createdByUserId !== userId,
|
||||
},
|
||||
description: `Deleted task share for task ${share.taskId}${
|
||||
orgRole === 'org:admin' && share.createdByUserId !== userId
|
||||
? ' (admin deletion)'
|
||||
: ''
|
||||
}`,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -238,29 +323,33 @@ export async function deleteTaskShare(shareId: string): Promise<ApiResponse> {
|
|||
*/
|
||||
export async function getTaskShares(taskId: string): Promise<TaskShare[]> {
|
||||
try {
|
||||
const { userId, orgId } = await auth();
|
||||
// Check if user can access this task (includes auth)
|
||||
const { canShare, error, userId, orgId, orgRole } =
|
||||
await canShareTask(taskId);
|
||||
|
||||
if (!userId || !orgId) {
|
||||
throw new Error('Authentication required');
|
||||
if (!canShare) {
|
||||
throw new Error(error || 'Task not found or access denied');
|
||||
}
|
||||
|
||||
const tasks = await getTasks({ orgId, userId });
|
||||
const task = tasks.find((t) => t.taskId === taskId);
|
||||
if (!userId || !orgId) {
|
||||
throw new Error('Authentication failed');
|
||||
}
|
||||
|
||||
if (!task) {
|
||||
throw new Error('Task not found or access denied');
|
||||
// For admins, show all shares for the task
|
||||
// For members, only show shares they created
|
||||
const whereConditions = [
|
||||
eq(taskShares.taskId, taskId),
|
||||
eq(taskShares.orgId, orgId),
|
||||
];
|
||||
|
||||
if (orgRole !== 'org:admin') {
|
||||
whereConditions.push(eq(taskShares.createdByUserId, userId));
|
||||
}
|
||||
|
||||
const shares = await db
|
||||
.select()
|
||||
.from(taskShares)
|
||||
.where(
|
||||
and(
|
||||
eq(taskShares.taskId, taskId),
|
||||
eq(taskShares.orgId, orgId),
|
||||
eq(taskShares.createdByUserId, userId),
|
||||
),
|
||||
)
|
||||
.where(and(...whereConditions))
|
||||
.orderBy(desc(taskShares.createdAt));
|
||||
|
||||
return shares.filter((share) => !isShareExpired(share.expiresAt));
|
||||
|
|
|
|||
|
|
@ -3,9 +3,11 @@ import { X } from 'lucide-react';
|
|||
|
||||
import type { TaskWithUser } from '@/actions/analytics';
|
||||
import { getMessages } from '@/actions/analytics';
|
||||
import { canShareTask } from '@/actions/taskSharing';
|
||||
import { useOrganizationSettings } from '@/hooks/useOrganizationSettings';
|
||||
import { formatCurrency, formatNumber } from '@/lib/formatters';
|
||||
import { generateFallbackTitle } from '@/lib/task-utils';
|
||||
import { QueryKey } from '@/types/react-query';
|
||||
import { Drawer, DrawerContent, Button } from '@/components/ui';
|
||||
import { ShareButton } from '@/components/task-sharing/ShareButton';
|
||||
|
||||
|
|
@ -25,16 +27,26 @@ export const TaskDrawer = ({ task, onClose }: TaskDrawerProps) => {
|
|||
|
||||
const { data: orgSettings } = useOrganizationSettings();
|
||||
|
||||
const { data: sharePermission } = useQuery({
|
||||
queryKey: [QueryKey.CanShareTask, task.taskId],
|
||||
queryFn: () => canShareTask(task.taskId),
|
||||
enabled: !!task.taskId,
|
||||
});
|
||||
|
||||
const isTaskSharingEnabled =
|
||||
orgSettings?.cloudSettings?.enableTaskSharing ?? false;
|
||||
|
||||
const canUserShareThisTask = sharePermission?.canShare ?? false;
|
||||
|
||||
return (
|
||||
<Drawer open={true} onOpenChange={onClose} direction="right">
|
||||
<DrawerContent className="flex flex-col h-full">
|
||||
<div className="flex-1 overflow-y-auto">
|
||||
<div className="p-4">
|
||||
<div className="flex justify-end gap-2 mb-4">
|
||||
{isTaskSharingEnabled && <ShareButton task={task} />}
|
||||
{isTaskSharingEnabled && canUserShareThisTask && (
|
||||
<ShareButton task={task} />
|
||||
)}
|
||||
<Button variant="ghost" size="sm" onClick={onClose}>
|
||||
<X className="size-4" />
|
||||
</Button>
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
export enum QueryKey {
|
||||
GetOrganizationSettings = 'GetOrganizationSettings',
|
||||
GetDynamicRouterModels = 'GetDynamicRouterModels',
|
||||
CanShareTask = 'canShareTask',
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue