Merge pull request #2538 from objectstack-ai/claude/attachments-v1-followups-8jhvin

fix(attachments): download via authenticated signed URL (framework #2970)

Author: os-zhuang

.changeset/attachment-authenticated-downloads.md

@@ -0,0 +1,13 @@
+---
+"@object-ui/app-shell": patch
+---
+
+fix(attachments): download attachments via authenticated signed URL (framework #2970)
+
+The framework now requires an authenticated session to download an
+attachments-scope file (the stable `/storage/files/:fileId` endpoint returns
+`401`/`403` for them). `RecordAttachmentsPanel`'s download control no longer
+uses a bare `<a href>` (which cannot carry the console's Bearer token) — it
+fetches a short-lived signed URL from `/storage/files/:fileId/url` with
+`createAuthenticatedFetch`, then opens it. `403 ATTACHMENT_DOWNLOAD_DENIED` and
+`401 AUTH_REQUIRED` map to friendly copy instead of a broken link.

packages/app-shell/src/views/RecordAttachmentsPanel.tsx

@@ -25,8 +25,9 @@ import { useObjectTranslation } from '@object-ui/react';
  * Storage model: one `sys_file` row per uploaded blob (three-step
  * presigned upload via @object-ui/providers' ObjectStack adapter), one
  * `sys_attachment` join row linking it to `(parent_object, parent_id)`.
- * Downloads go through the stable `/storage/files/:fileId` endpoint,
- * which 302-redirects to a freshly signed URL on every request.
+ * Downloads fetch a short-lived signed URL from `/storage/files/:fileId/url`
+ * with the console's Bearer token (the endpoint requires an authenticated
+ * session for attachments-scope files, #2970), then open it.
  */
 
 interface AttachmentRow {
@@ -74,16 +75,16 @@ export const RecordAttachmentsPanel: React.FC<RecordAttachmentsPanelProps> = ({
   // Vite dev console proxies same-origin `/api` unless VITE_SERVER_URL
   // points elsewhere.
   const baseUrl = (import.meta as any).env?.VITE_SERVER_URL || '';
+  // One authenticated fetch (Bearer token from localStorage) reused for the
+  // upload adapter and the download-URL fetch — the storage routes require a
+  // session and there is no cookie for `credentials: 'include'` to carry.
+  const authFetch = React.useMemo(() => createAuthenticatedFetch(), []);
   const adapter = React.useMemo(
-    // `fetchImpl`: the storage upload routes require an authenticated
-    // session when the server has an auth service (#2755), and the console
-    // authenticates with a Bearer token (localStorage) — there is no session
-    // cookie for `credentials: 'include'` to carry.
-    () => createObjectStackUploadAdapter({ baseUrl, scope: 'attachments', fetchImpl: createAuthenticatedFetch() }),
-    [baseUrl],
+    () => createObjectStackUploadAdapter({ baseUrl, scope: 'attachments', fetchImpl: authFetch }),
+    [baseUrl, authFetch],
   );
 
-  /** Map the server's fail-closed 403 codes (#2755) to friendly copy. */
+  /** Map the server's fail-closed 40x codes (#2755, #2970) to friendly copy. */
   const friendlyError = React.useCallback(
     (err: unknown): string => {
       const anyErr = err as { code?: string; message?: unknown } | null;
@@ -99,6 +100,16 @@ export const RecordAttachmentsPanel: React.FC<RecordAttachmentsPanelProps> = ({
           defaultValue: "You don't have access to attach files to this record.",
         });
       }
+      if (has('ATTACHMENT_DOWNLOAD_DENIED')) {
+        return t('detail.attachmentDownloadDenied', {
+          defaultValue: "You don't have access to download this attachment.",
+        });
+      }
+      if (has('AUTH_REQUIRED')) {
+        return t('detail.attachmentAuthRequired', {
+          defaultValue: 'Please sign in to download this attachment.',
+        });
+      }
       if (has('PERMISSION_DENIED')) {
         return t('detail.attachmentPermissionDenied', {
           defaultValue: "You don't have permission to do that.",
@@ -186,6 +197,38 @@ export const RecordAttachmentsPanel: React.FC<RecordAttachmentsPanelProps> = ({
     [dataSource, friendlyError],
   );
 
+  const handleDownload = React.useCallback(
+    async (row: AttachmentRow) => {
+      setError(null);
+      try {
+        // The stable `/files/:fileId` endpoint now requires an authenticated
+        // session for attachments-scope files (#2970) — an <a href> can't
+        // carry the Bearer token. Fetch a short-lived signed URL with auth,
+        // then open it (the signed URL itself needs no credentials).
+        const res = await authFetch(
+          `${baseUrl}/api/v1/storage/files/${encodeURIComponent(row.file_id)}/url`,
+        );
+        if (!res.ok) {
+          let code: string | undefined;
+          try {
+            code = (await res.json())?.code;
+          } catch {
+            /* non-JSON body */
+          }
+          throw Object.assign(new Error(code ?? `Download failed (${res.status})`), { code });
+        }
+        const body = await res.json();
+        const url: string | undefined = body?.url ?? body?.data?.url;
+        if (!url) throw new Error('Download URL missing from response');
+        const target = /^https?:/i.test(url) ? url : `${baseUrl}${url}`;
+        window.open(target, '_blank', 'noopener,noreferrer');
+      } catch (err: any) {
+        setError(friendlyError(err));
+      }
+    },
+    [authFetch, baseUrl, friendlyError],
+  );
+
   return (
     <div className={cn('rounded-lg border bg-card', className)} data-testid="record-attachments-panel">
       <div className="flex items-center justify-between px-4 py-3 border-b">
@@ -248,17 +291,15 @@ export const RecordAttachmentsPanel: React.FC<RecordAttachmentsPanelProps> = ({
                     .join(' · ')}
                 </div>
               </div>
-              <a
-                href={`${baseUrl}/api/v1/storage/files/${encodeURIComponent(row.file_id)}`}
-                target="_blank"
-                rel="noreferrer"
-                className="inline-flex"
+              <Button
+                variant="ghost"
+                size="icon"
+                className="h-8 w-8"
                 aria-label={t('detail.downloadAttachment', { defaultValue: 'Download' })}
+                onClick={() => void handleDownload(row)}
               >
-                <Button variant="ghost" size="icon" className="h-8 w-8">
-                  <Download className="h-4 w-4" />
-                </Button>
-              </a>
+                <Download className="h-4 w-4" />
+              </Button>
               <Button
                 variant="ghost"
                 size="icon"

packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx

@@ -93,3 +93,70 @@ describe('RecordAttachmentsPanel — server-denial error mapping (#2755)', () =>
     expect(dataSource.delete).toHaveBeenCalledWith('sys_attachment', 'a1');
   });
 });
+
+describe('RecordAttachmentsPanel — authenticated signed-URL download (#2970)', () => {
+  it('fetches /files/:id/url with auth and opens the signed URL', async () => {
+    const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
+    const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(
+      new Response(JSON.stringify({ url: '/api/v1/storage/_local/raw/tok123' }), {
+        status: 200,
+        headers: { 'Content-Type': 'application/json' },
+      }),
+    );
+    setup(makeDataSource());
+    await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument());
+
+    await userEvent.setup().click(screen.getByRole('button', { name: 'Download' }));
+
+    await waitFor(() => expect(fetchSpy).toHaveBeenCalled());
+    const calledUrl = String(fetchSpy.mock.calls[0][0]);
+    expect(calledUrl).toContain('/api/v1/storage/files/f1/url');
+    await waitFor(() =>
+      expect(openSpy).toHaveBeenCalledWith(
+        expect.stringContaining('/api/v1/storage/_local/raw/tok123'),
+        '_blank',
+        'noopener,noreferrer',
+      ),
+    );
+    expect(screen.queryByRole('alert')).not.toBeInTheDocument();
+  });
+
+  it('maps a 403 ATTACHMENT_DOWNLOAD_DENIED to friendly copy and does not open a tab', async () => {
+    const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
+    vi.spyOn(globalThis, 'fetch').mockResolvedValue(
+      new Response(JSON.stringify({ code: 'ATTACHMENT_DOWNLOAD_DENIED' }), {
+        status: 403,
+        headers: { 'Content-Type': 'application/json' },
+      }),
+    );
+    setup(makeDataSource());
+    await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument());
+
+    await userEvent.setup().click(screen.getByRole('button', { name: 'Download' }));
+
+    await waitFor(() =>
+      expect(screen.getByRole('alert')).toHaveTextContent(
+        "You don't have access to download this attachment.",
+      ),
+    );
+    expect(openSpy).not.toHaveBeenCalled();
+  });
+
+  it('maps a 401 AUTH_REQUIRED to friendly copy', async () => {
+    vi.spyOn(window, 'open').mockImplementation(() => null);
+    vi.spyOn(globalThis, 'fetch').mockResolvedValue(
+      new Response(JSON.stringify({ code: 'AUTH_REQUIRED' }), {
+        status: 401,
+        headers: { 'Content-Type': 'application/json' },
+      }),
+    );
+    setup(makeDataSource());
+    await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument());
+
+    await userEvent.setup().click(screen.getByRole('button', { name: 'Download' }));
+
+    await waitFor(() =>
+      expect(screen.getByRole('alert')).toHaveTextContent('Please sign in to download this attachment.'),
+    );
+  });
+});