diff --git a/apps/server/src/api/v2/reports/[id]/GET.ts b/apps/server/src/api/v2/reports/[id]/GET.ts index 1fa5ef644..b50167d86 100644 --- a/apps/server/src/api/v2/reports/[id]/GET.ts +++ b/apps/server/src/api/v2/reports/[id]/GET.ts @@ -1,3 +1,4 @@ +import { checkPermission } from '@buster/access-controls'; import { getReportFileById } from '@buster/database/queries'; import { GetReportParamsSchema, @@ -7,7 +8,6 @@ import { import { zValidator } from '@hono/zod-validator'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; -import { checkAssetPublicAccess } from '../../../../shared-helpers/asset-public-access'; import { standardErrorHandler } from '../../../../utils/response'; export async function getReportHandler( @@ -22,15 +22,29 @@ export async function getReportHandler( versionNumber, }); - return await checkAssetPublicAccess({ - user, + const permission = await checkPermission({ + userId: user.id, assetId: reportId, assetType: 'report_file', - organizationId: report.organization_id, + requiredRole: 'can_view', workspaceSharing: report.workspace_sharing, - password, - asset: report, + organizationId: report.organization_id, + publiclyAccessible: report.publicly_accessible, + publicExpiryDate: report.public_expiry_date ?? undefined, + publicPassword: report.public_password ?? undefined, + userSuppliedPassword: password, }); + + if (!permission.hasAccess || !permission.effectiveRole) { + throw new HTTPException(403, { message: 'You do not have permission to view this report' }); + } + + const response: GetReportResponse = { + ...report, + permission: permission.effectiveRole, + }; + + return response; } const app = new Hono() diff --git a/apps/server/src/shared-helpers/asset-public-access.test.ts b/apps/server/src/shared-helpers/asset-public-access.test.ts deleted file mode 100644 index 19fe29898..000000000 --- a/apps/server/src/shared-helpers/asset-public-access.test.ts +++ /dev/null @@ -1,494 +0,0 @@ -import { type WorkspaceSharing, checkPermission } from '@buster/access-controls'; -import type { AssetType } from '@buster/server-shared/assets'; -import type { ShareUpdateRequest } from '@buster/server-shared/share'; -import { HTTPException } from 'hono/http-exception'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Mock } from 'vitest'; -import { checkAssetPublicAccess } from './asset-public-access'; - -// Mock the checkPermission function -vi.mock('@buster/access-controls', () => ({ - checkPermission: vi.fn(), -})); - -describe('checkAssetPublicAccess', () => { - const mockCheckPermission = checkPermission as Mock; - - // Mock data setup - const mockUser = { - id: 'user-123', - }; - - const mockAssetId = 'asset-123'; - const mockAssetType: AssetType = 'report_file'; - const mockOrganizationId = 'org-123'; - const mockWorkspaceSharing: WorkspaceSharing = 'can_view'; - - const baseAsset: Pick< - ShareUpdateRequest, - 'publicly_accessible' | 'public_expiry_date' | 'public_password' - > = { - publicly_accessible: false, - public_expiry_date: null, - public_password: null, - }; - - const commonParams = { - user: mockUser, - assetId: mockAssetId, - assetType: mockAssetType, - organizationId: mockOrganizationId, - workspaceSharing: mockWorkspaceSharing, - }; - - beforeEach(() => { - vi.clearAllMocks(); - }); - - describe('when user has permission access', () => { - beforeEach(() => { - mockCheckPermission.mockResolvedValue({ hasAccess: true }); - }); - - it('should return asset when user has view permission', async () => { - const asset = { ...baseAsset, publicly_accessible: false }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }); - - expect(result).toEqual(asset); - expect(mockCheckPermission).toHaveBeenCalledWith({ - userId: mockUser.id, - assetId: mockAssetId, - assetType: mockAssetType, - requiredRole: 'can_view', - organizationId: mockOrganizationId, - workspaceSharing: mockWorkspaceSharing, - publiclyAccessible: false, - publicExpiryDate: undefined, - publicPassword: undefined, - userSuppliedPassword: undefined, - }); - }); - - it('should return asset when user has custom required role', async () => { - const asset = { ...baseAsset }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - requiredRole: 'can_edit', - }); - - expect(result).toEqual(asset); - expect(mockCheckPermission).toHaveBeenCalledWith({ - userId: mockUser.id, - assetId: mockAssetId, - assetType: mockAssetType, - requiredRole: 'can_edit', - organizationId: mockOrganizationId, - workspaceSharing: mockWorkspaceSharing, - publiclyAccessible: false, - publicExpiryDate: undefined, - publicPassword: undefined, - userSuppliedPassword: undefined, - }); - }); - - it('should return asset even when it has public access settings if user has permission', async () => { - const asset = { - ...baseAsset, - publicly_accessible: true, - public_password: 'secret', - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, // No password provided, but should work since user has permission - }); - - expect(result).toEqual(asset); - }); - }); - - describe('when user does not have permission access', () => { - beforeEach(() => { - mockCheckPermission.mockResolvedValue({ hasAccess: false }); - }); - - describe('and asset is not publicly accessible', () => { - it('should throw 403 error', async () => { - const asset = { ...baseAsset, publicly_accessible: false }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }) - ).rejects.toThrow( - new HTTPException(403, { message: 'You do not have permission to view this report' }) - ); - }); - }); - - describe('and asset is publicly accessible', () => { - it('should return asset when no expiry date and no password', async () => { - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: null, - public_password: null, - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }); - - expect(result).toEqual(asset); - }); - - it('should return asset when expiry date is in the future', async () => { - const futureDate = new Date(); - futureDate.setDate(futureDate.getDate() + 1); // Tomorrow - - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: futureDate.toISOString(), - public_password: null, - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }); - - expect(result).toEqual(asset); - }); - - it('should throw 403 error when expiry date is in the past', async () => { - const pastDate = new Date(); - pastDate.setDate(pastDate.getDate() - 1); // Yesterday - - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: pastDate.toISOString(), - public_password: null, - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }) - ).rejects.toThrow( - new HTTPException(403, { message: 'Public access to this report has expired' }) - ); - }); - - it('should throw 418 error when password is required but not provided', async () => { - const asset = { - ...baseAsset, - publicly_accessible: true, - public_password: 'secret123', - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }) - ).rejects.toThrow( - new HTTPException(418, { message: 'Password required for public access' }) - ); - }); - - it('should throw 403 error when wrong password is provided', async () => { - const asset = { - ...baseAsset, - publicly_accessible: true, - public_password: 'secret123', - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: 'wrong-password', - }) - ).rejects.toThrow( - new HTTPException(403, { message: 'Password required for public access' }) - ); - }); - - it('should return asset when correct password is provided', async () => { - const asset = { - ...baseAsset, - publicly_accessible: true, - public_password: 'secret123', - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: 'secret123', - }); - - expect(result).toEqual(asset); - }); - - it('should handle combined expiry date and password correctly - valid case', async () => { - const futureDate = new Date(); - futureDate.setDate(futureDate.getDate() + 1); // Tomorrow - - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: futureDate.toISOString(), - public_password: 'secret123', - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: 'secret123', - }); - - expect(result).toEqual(asset); - }); - - it('should throw 403 error when expiry date is past even with correct password', async () => { - const pastDate = new Date(); - pastDate.setDate(pastDate.getDate() - 1); // Yesterday - - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: pastDate.toISOString(), - public_password: 'secret123', - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: 'secret123', - }) - ).rejects.toThrow( - new HTTPException(403, { message: 'Public access to this report has expired' }) - ); - }); - - it('should throw 418 error when expiry is valid but password is missing', async () => { - const futureDate = new Date(); - futureDate.setDate(futureDate.getDate() + 1); // Tomorrow - - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: futureDate.toISOString(), - public_password: 'secret123', - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }) - ).rejects.toThrow( - new HTTPException(418, { message: 'Password required for public access' }) - ); - }); - - it('should handle empty string password correctly', async () => { - const asset = { - ...baseAsset, - publicly_accessible: true, - public_password: '', - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: '', - }); - - expect(result).toEqual(asset); - }); - }); - - describe('edge cases', () => { - it('should handle asset with additional properties correctly', async () => { - const assetWithExtraProps = { - ...baseAsset, - publicly_accessible: true, - id: 'some-id', - name: 'Test Asset', - description: 'Test description', - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset: assetWithExtraProps, - password: undefined, - }); - - expect(result).toEqual(assetWithExtraProps); - }); - - it('should handle Date objects for expiry date correctly', async () => { - const futureDate = new Date(); - futureDate.setDate(futureDate.getDate() + 1); - - const asset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: futureDate.toISOString(), - }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }); - - expect(result).toEqual(asset); - }); - - it('should handle different asset types correctly', async () => { - const asset = { ...baseAsset, publicly_accessible: true }; - - const result = await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - assetType: 'dashboard' as AssetType, - }); - - expect(result).toEqual(asset); - expect(mockCheckPermission).toHaveBeenCalledWith({ - userId: mockUser.id, - assetId: mockAssetId, - assetType: 'dashboard', - requiredRole: 'can_view', - organizationId: mockOrganizationId, - workspaceSharing: mockWorkspaceSharing, - publiclyAccessible: true, - publicExpiryDate: undefined, - publicPassword: undefined, - userSuppliedPassword: undefined, - }); - }); - }); - - describe('error message consistency', () => { - it('should have consistent error messages for different scenarios', async () => { - // Test that all permission-related errors use the same base message - const asset = { ...baseAsset, publicly_accessible: false }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }) - ).rejects.toThrow('You do not have permission to view this report'); - - // Test expired access message - const pastDate = new Date(); - pastDate.setDate(pastDate.getDate() - 1); - const expiredAsset = { - ...baseAsset, - publicly_accessible: true, - public_expiry_date: pastDate.toISOString(), - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset: expiredAsset, - password: undefined, - }) - ).rejects.toThrow('Public access to this report has expired'); - - // Test password required messages - const passwordProtectedAsset = { - ...baseAsset, - publicly_accessible: true, - public_password: 'secret', - }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset: passwordProtectedAsset, - password: undefined, - }) - ).rejects.toThrow('Password required for public access'); - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset: passwordProtectedAsset, - password: 'wrong', - }) - ).rejects.toThrow('Password required for public access'); - }); - }); - }); - - describe('checkPermission integration', () => { - it('should handle checkPermission promise rejection gracefully', async () => { - const permissionError = new Error('Database connection failed'); - mockCheckPermission.mockRejectedValue(permissionError); - - const asset = { ...baseAsset }; - - await expect( - checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - }) - ).rejects.toThrow('Database connection failed'); - }); - - it('should pass all required parameters to checkPermission', async () => { - mockCheckPermission.mockResolvedValue({ hasAccess: true }); - const asset = { ...baseAsset }; - - await checkAssetPublicAccess({ - ...commonParams, - asset, - password: undefined, - requiredRole: 'can_edit', - }); - - expect(mockCheckPermission).toHaveBeenCalledWith({ - userId: 'user-123', - assetId: 'asset-123', - assetType: 'report_file', - requiredRole: 'can_edit', - organizationId: 'org-123', - workspaceSharing: 'can_view', - publiclyAccessible: false, - publicExpiryDate: undefined, - publicPassword: undefined, - userSuppliedPassword: undefined, - }); - }); - }); -}); diff --git a/apps/server/src/shared-helpers/asset-public-access.ts b/apps/server/src/shared-helpers/asset-public-access.ts index 5833e10ed..75a596d11 100644 --- a/apps/server/src/shared-helpers/asset-public-access.ts +++ b/apps/server/src/shared-helpers/asset-public-access.ts @@ -5,70 +5,8 @@ import { } from '@buster/access-controls'; import { getUserOrganizationId } from '@buster/database/queries'; import type { AssetType } from '@buster/server-shared/assets'; -import type { ShareUpdateRequest } from '@buster/server-shared/share'; import { HTTPException } from 'hono/http-exception'; -// Base interface for assets with public access properties -type PublicAccessAsset = Pick< - ShareUpdateRequest, - 'publicly_accessible' | 'public_expiry_date' | 'public_password' ->; - -export const checkAssetPublicAccess = async ({ - user, - assetId, - assetType, - requiredRole = 'can_view', - organizationId, - workspaceSharing, - asset, - password, -}: { - user: { - id: string; - }; - assetId: string; - assetType: AssetType; - requiredRole?: AssetPermissionRole; - organizationId: string; - workspaceSharing: WorkspaceSharing; - password: string | undefined; - asset: T; -}): Promise => { - const assetPermissionResult = await checkPermission({ - userId: user.id, - assetId, - assetType, - requiredRole, - organizationId, - workspaceSharing, - publiclyAccessible: asset.publicly_accessible ?? false, - publicExpiryDate: asset.public_expiry_date ?? undefined, - publicPassword: asset.public_password ?? undefined, - userSuppliedPassword: password, - }); - - if (!assetPermissionResult.hasAccess) { - const now = new Date(); - if (asset.publicly_accessible) { - if (asset.public_expiry_date && new Date(asset.public_expiry_date) < now) { - throw new HTTPException(403, { message: 'Public access to this report has expired' }); - } - if (asset.public_password && !password) { - throw new HTTPException(418, { message: 'Password required for public access' }); - } - if (asset.public_password && asset.public_password !== password) { - throw new HTTPException(403, { message: 'Password required for public access' }); - } - // If we get here, public access is valid - return asset; - } - throw new HTTPException(403, { message: 'You do not have permission to view this report' }); - } - - return asset; -}; - export const checkIfAssetIsEditable = async ({ user, assetId, diff --git a/apps/trigger/src/tasks/analyst-agent-task/analyst-agent-task.ts b/apps/trigger/src/tasks/analyst-agent-task/analyst-agent-task.ts index a35bebb99..93d690c12 100644 --- a/apps/trigger/src/tasks/analyst-agent-task/analyst-agent-task.ts +++ b/apps/trigger/src/tasks/analyst-agent-task/analyst-agent-task.ts @@ -137,11 +137,11 @@ class ResourceTracker { }, cpuUsage: finalCpuUsage ? { - userTimeMs: Math.round(finalCpuUsage.user / 1000), - systemTimeMs: Math.round(finalCpuUsage.system / 1000), - totalTimeMs: Math.round(totalCpuTime / 1000), - estimatedUsagePercent: Math.round(cpuPercentage * 100) / 100, - } + userTimeMs: Math.round(finalCpuUsage.user / 1000), + systemTimeMs: Math.round(finalCpuUsage.system / 1000), + totalTimeMs: Math.round(totalCpuTime / 1000), + estimatedUsagePercent: Math.round(cpuPercentage * 100) / 100, + } : { error: 'CPU usage not available' }, stageBreakdown: this.snapshots.map((snapshot, index) => { const prevSnapshot = index > 0 ? this.snapshots[index - 1] : null; @@ -253,7 +253,7 @@ export const analystAgentTask: ReturnType< queue: analystQueue, maxDuration: 1200, // 20 minutes for complex analysis retry: { - maxAttempts: 0 + maxAttempts: 0, }, onFailure: async ({ error, payload }) => { // Log the failure for debugging @@ -424,12 +424,12 @@ export const analystAgentTask: ReturnType< conversationHistory.length > 0 ? conversationHistory : [ - { - role: 'user', - // v5 supports string content directly for user messages - content: messageContext.requestMessage, - }, - ]; + { + role: 'user', + // v5 supports string content directly for user messages + content: messageContext.requestMessage, + }, + ]; const workflowInput: AnalystWorkflowInput = { messages: modelMessages, diff --git a/packages/database/src/queries/reports/get-report.ts b/packages/database/src/queries/reports/get-report.ts index 6e155f834..84e501077 100644 --- a/packages/database/src/queries/reports/get-report.ts +++ b/packages/database/src/queries/reports/get-report.ts @@ -98,13 +98,11 @@ export async function getReportFileById(input: GetReportInput) { reportCollectionsResult, individualPermissionsResult, workspaceMemberCount, - userPermission, ] = await Promise.all([ reportDataQuery, isOrganizationMember ? reportCollectionsQuery : Promise.resolve([]), isOrganizationMember ? individualPermissionsQuery : Promise.resolve([]), isOrganizationMember ? getOrganizationMemberCount(organizationId) : Promise.resolve(0), - getAssetPermission(userId, reportId, 'report_file'), ]); const reportData = reportDataResult[0]; @@ -137,7 +135,6 @@ export async function getReportFileById(input: GetReportInput) { versions: versionHistoryArray, collections: reportCollectionsResult, individual_permissions: individualPermissionsResult, - permission: userPermission ?? 'can_view', workspace_member_count: workspaceMemberCount, };