Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ jobs:
lib/knowledge/__integration__/search-reference-batching.integration.ts
lib/core/outbox/service.integration.ts
lib/knowledge/__integration__/connector-upload.integration.ts
lib/uploads/contexts/organization-logo/application.integration.ts

test-build:
name: Lint and Test
Expand Down
16 changes: 16 additions & 0 deletions apps/sim/app/api/files/authorization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,22 @@ describe('public-context access (profile-pictures / og-images / workspace-logos)
return verifyFileAccess(cloudKey, USER_ID, undefined, context, false, { requireWrite: true })
}

it('allows organization logo reads and denies generic deletes even for the uploader', async () => {
const key = 'organization-logos/org-1/logo.png'
mockGetFileMetadata.mockResolvedValue({ userId: USER_ID })
await expect(verifyFileAccess(key, USER_ID, undefined, 'organization-logos')).resolves.toBe(
true
)
await expect(
verifyFileAccess(key, USER_ID, undefined, 'organization-logos', false, { requireWrite: true })
).resolves.toBe(false)
await expect(
verifyFileAccess(key, USER_ID, undefined, 'general', false, { requireWrite: true })
).resolves.toBe(false)
expect(mockGetFileMetadata).not.toHaveBeenCalled()
expect(mockGetUserEntityPermissions).not.toHaveBeenCalled()
})

it('grants public reads without any ownership check', async () => {
await expect(read('og-images/banner.png', 'og-images')).resolves.toBe(true)
await expect(read('profile-pictures/123-avatar.png', 'profile-pictures')).resolves.toBe(true)
Expand Down
2 changes: 2 additions & 0 deletions apps/sim/app/api/files/authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ export async function verifyFileAccess(
const requireWrite = options?.requireWrite ?? false
try {
const keyContext = inferContextFromKey(cloudKey)
/** Organization logos are changed only through the organization-authorized upload lifecycle. */
if (keyContext === 'organization-logos') return !requireWrite
if (keyContext === 'knowledge-base') {
return requireWrite
? verifyKBFileWriteAccess(cloudKey, userId)
Expand Down
22 changes: 22 additions & 0 deletions apps/sim/app/api/files/serve/[...path]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,28 @@ describe('File Serve API Route', () => {
})
})

it('serves organization logos through the existing public asset path', async () => {
mockIsUsingCloudStorage.mockReturnValue(true)
mockInferContextFromKey.mockReturnValue('organization-logos')
const key = 'organization-logos/org-1/upload-1-logo.png'
const response = await GET(new NextRequest(`http://localhost/api/files/serve/s3/${key}`), {
params: Promise.resolve({ path: ['s3', 'organization-logos', 'org-1', 'upload-1-logo.png'] }),
})
expect(response.status).toBe(200)
expect(storageServiceMockFns.mockDownloadFile).toHaveBeenCalledWith({
key,
context: 'organization-logos',
maxBytes: MAX_BUFFERED_TRANSFER_BYTES,
})
expect(mockCreateFileResponse).toHaveBeenCalledWith(
expect.objectContaining({
cacheControl: 'public, max-age=31536000',
})
)
expect(mockVerifyFileAccess).not.toHaveBeenCalled()
expect(mockAuthenticateWorkspaceFile).not.toHaveBeenCalled()
})

it('should return 404 when file not found', async () => {
mockVerifyFileAccess.mockResolvedValue(false)
mockFindLocalFile.mockReturnValue(null)
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/app/api/files/serve/[...path]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ export const GET = withRouteHandler(
const isPublicByKeyPrefix =
cloudKey.startsWith('profile-pictures/') ||
cloudKey.startsWith('og-images/') ||
cloudKey.startsWith('workspace-logos/')
cloudKey.startsWith('workspace-logos/') ||
cloudKey.startsWith('organization-logos/')

if (isPublicByKeyPrefix) {
const context = inferContextFromKey(cloudKey)
Expand Down
8 changes: 8 additions & 0 deletions apps/sim/app/api/files/uploads/finalizers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import { captureServerEvent } from '@/lib/posthog/server'
import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify'
import { getServeStoragePrefix } from '@/lib/uploads/config'
import { finalizeOrganizationAssistantAttachment } from '@/lib/uploads/contexts/organization-assistant/application'
import {
finalizeOrganizationLogoUpload,
organizationLogoUploadResult,
} from '@/lib/uploads/contexts/organization-logo/application'
import {
getWorkspaceFile,
registerUploadedWorkspaceFile,
Expand Down Expand Up @@ -106,6 +110,8 @@ export async function finalizeUploadPurpose({
)
case 'profile_picture':
return { value: storedAssetResult(session, 'profile-pictures') }
case 'organization_logo':
return finalizeOrganizationLogoUpload(principal, session, request)
case 'workspace_logo':
return finalizeWorkspaceLogo(session, actor, request)
case 'mothership_attachment':
Expand Down Expand Up @@ -137,6 +143,8 @@ export async function loadCompletedUploadPurpose(
switch (session.purpose) {
case 'workspace_file':
return toV2File(await loadCompletedWorkspaceFileUpload(session))
case 'organization_logo':
return organizationLogoUploadResult(session)
case 'profile_picture':
case 'workspace_logo':
case 'mothership_attachment':
Expand Down
11 changes: 11 additions & 0 deletions apps/sim/app/api/files/uploads/purposes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const INTERNAL_UPLOAD_PURPOSES = new Set<InternalUploadPurpose>([
'workspace_file',
'profile_picture',
'workspace_logo',
'organization_logo',
'mothership_attachment',
'execution_attachment',
])
Expand Down Expand Up @@ -57,6 +58,11 @@ export async function createPurposeUploadSession(
localOrigin,
})
}
case 'organization_logo':
throw new UploadSessionError(
'validation',
'Organization logos require organization authorization'
)
case 'profile_picture':
return createUploadSession({
purpose: body.purpose,
Expand Down Expand Up @@ -121,6 +127,11 @@ export async function reauthorizeUploadPurpose(
case 'mothership_attachment':
await requireWorkspacePermission(userId, requireSessionScope(session.workspaceId), 'write')
return
case 'organization_logo':
throw new UploadSessionError(
'forbidden',
'Organization logos require organization authorization'
)
case 'profile_picture':
return
case 'workspace_logo':
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
'use client'

import {
ChipInput,
chipVariants,
cn,
DropdownMenuItem,
Loader,
OverflowText,
Skeleton,
} from '@sim/emcn'
import { chipVariants, cn, DropdownMenuItem, Loader, OverflowText, Skeleton } from '@sim/emcn'
import { MoreHorizontal, Pin, Task } from '@sim/emcn/icons'
import type { OrganizationChat } from '@/app/o/[organizationId]/components/organization-sidebar/hooks'
import { useOrganizationChatActions } from '@/app/o/[organizationId]/components/organization-sidebar/hooks/use-organization-chat-actions'
Expand All @@ -18,6 +10,7 @@ import {
CollapsedSidebarMenu,
SidebarSection,
} from '@/app/workspace/[workspaceId]/w/components/sidebar/components'
import { SidebarRenameRow } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-rename-row'
import { ContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu'
import { DeleteModal } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/delete-modal/delete-modal'
import {
Expand Down Expand Up @@ -187,7 +180,7 @@ export function ChatsSection({
)}
{chats.map((chat) =>
rename.editingId === chat.id ? (
<ChipInput
<SidebarRenameRow
key={chat.id}
ref={rename.inputRef}
aria-label={`Rename chat ${chat.name}`}
Expand All @@ -196,8 +189,6 @@ export function ChatsSection({
onKeyDown={rename.handleKeyDown}
onBlur={saveRename}
disabled={rename.isSaving}
maxLength={100}
autoComplete='off'
/>
) : (
<ChatRow
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/**
* @vitest-environment jsdom
*/
import { act } from 'react'
import { ToastProvider } from '@sim/emcn'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

const mocks = vi.hoisted(() => ({ upload: vi.fn(), refresh: vi.fn() }))
vi.mock('@/lib/uploads/client/session-upload', () => ({
uploadInternalFileSession: mocks.upload,
}))
vi.mock('next/navigation', () => ({
useRouter: () => ({ refresh: mocks.refresh }),
usePathname: () => '/o/org-1/home',
}))

import { OrganizationHeader } from '@/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header'
import { organizationKeys } from '@/hooks/queries/utils/organization-keys'

const organization = { id: 'org-1', name: 'Design', slug: 'design', logo: null, memberCount: 2 }
let root: Root
let container: HTMLDivElement
let queryClient: QueryClient

beforeEach(() => {
vi.clearAllMocks()
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
vi.stubGlobal(
'ResizeObserver',
class {
observe() {}
unobserve() {}
disconnect() {}
}
)
mocks.upload.mockResolvedValue({ path: '/api/files/serve/organization-logos/logo.png' })
queryClient = new QueryClient({ defaultOptions: { mutations: { retry: false } } })
container = document.createElement('div')
document.body.append(container)
root = createRoot(container)
})

afterEach(async () => {
await act(async () => root.unmount())
container.remove()
queryClient.clear()
vi.unstubAllGlobals()
})

async function render(canEditLogo = true) {
await act(async () => {
root.render(
<QueryClientProvider client={queryClient}>
<ToastProvider>
<OrganizationHeader
organization={organization}
canEditLogo={canEditLogo}
isCollapsed={false}
onExpandSidebar={vi.fn()}
/>
</ToastProvider>
</QueryClientProvider>
)
})
}

async function openMenu() {
await act(async () => {
container
.querySelector('[aria-label="Organization menu"]')!
.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, button: 0 }))
})
}

function menuItem(name: string) {
return Array.from(document.querySelectorAll<HTMLElement>('[role="menuitem"]')).find(
(item) => item.textContent === name
)
}

async function pickFile(file: File) {
const input = container.querySelector<HTMLInputElement>('input[type="file"]')!
Object.defineProperty(input, 'files', { configurable: true, value: [file] })
await act(async () => input.dispatchEvent(new Event('change', { bubbles: true })))
}

describe('OrganizationHeader logo upload', () => {
it('opens the same native file picker from the admin menu', async () => {
await render()
await openMenu()
const input = container.querySelector<HTMLInputElement>('input[type="file"]')!
const click = vi.spyOn(input, 'click').mockImplementation(() => {})
expect(input.accept).toContain('image/png')
await act(async () => menuItem('Upload logo')!.click())
expect(click).toHaveBeenCalledOnce()
})

it('does not offer logo changes to members', async () => {
await render(false)
await openMenu()
expect(menuItem('Upload logo')).toBeUndefined()
expect(container.querySelector('input[type="file"]')).toBeNull()
})

it('uploads under the organization scope and refreshes its identity after success', async () => {
await render()
const invalidate = vi.spyOn(queryClient, 'invalidateQueries')
const file = new File(['image'], 'logo.png', { type: 'image/png' })
await pickFile(file)
expect(mocks.upload).toHaveBeenCalledWith({
purpose: 'organization_logo',
organizationId: organization.id,
file,
})
expect(invalidate).toHaveBeenCalledWith({ queryKey: organizationKeys.detail('org-1') })
expect(invalidate).toHaveBeenCalledWith({ queryKey: organizationKeys.lists() })
expect(mocks.refresh).toHaveBeenCalledOnce()
})

it('rejects unsupported files before uploading', async () => {
await render()
await pickFile(new File(['text'], 'notes.txt', { type: 'text/plain' }))
expect(mocks.upload).not.toHaveBeenCalled()
expect(mocks.refresh).not.toHaveBeenCalled()
expect(document.body.textContent).toContain('not a supported image format')
})

it('keeps the saved identity when upload fails and allows retrying the same file', async () => {
const file = new File(['image'], 'logo.png', { type: 'image/png' })
mocks.upload.mockRejectedValueOnce(new Error('Upload failed'))
await render()
await pickFile(file)
expect(mocks.refresh).not.toHaveBeenCalled()
expect(document.body.textContent).toContain('Upload failed')
expect(container.querySelector<HTMLInputElement>('input[type="file"]')!.value).toBe('')
await pickFile(file)
expect(mocks.upload).toHaveBeenCalledTimes(2)
expect(mocks.refresh).toHaveBeenCalledOnce()
})
})
Loading
Loading