fix: hardening pass — permissions, socket reconnect, canvas setup, arrangements

- Fix 403 on save for public collection viewers (return role in GET board response) - Add read-only status indicator (StatusBar + StatusIndicator) - Fix beforeunload save to use fetch+keepalive with auth header - Socket reconnect now rejoins board room automatically - Canvas setup uses polling instead of brittle 200ms timer - Fix double user:left on disconnect (use disconnecting event, snapshot rooms) - Thread + comment creation wrapped in db.transaction - Prevent owner downgrade via addCollectionMember (check existing member) - Bound redirect depth in downloadImage to 5 - Arrangement operations anchor to bounding box top-left (no drift) - Distribute H/V also anchor to top-left - Fix annotations fetch to use axios api instance (401 interceptor) - Replace require() with static import in shortcut-definitions
2026-03-11 08:08:21 +05:30
parent fc2d9df741
commit 6518ed6763
13 changed files with 141 additions and 84 deletions
@@ -135,15 +135,15 @@ export function alignBottom(objects: SceneItem[]) {
 /** Distribute horizontally: normalize all to same height, then space evenly in a row. */
 export function distributeHorizontal(objects: SceneItem[]) {
  if (objects.length < 2) return;
+  const { x: startX, y: startY } = anchorTopLeft(objects);
  // Normalize heights first (uniform row)
  normalizeHeight(objects);
  // Then arrange as row with even spacing
  const gap = 20;
  const sorted = [...objects].sort((a, b) => a.data.x - b.data.x);
-  const startY = sorted[0].data.y;
-  let x = sorted[0].data.x;
+  let x = 0;
  sorted.forEach((item) => {
-    item.data.x = x;
+    item.data.x = startX + x;
    item.data.y = startY;
    syncPosition(item);
    x += scaledW(item) + gap;
@@ -153,16 +153,16 @@ export function distributeHorizontal(objects: SceneItem[]) {
 /** Distribute vertically: normalize all to same width, then space evenly in a column. */
 export function distributeVertical(objects: SceneItem[]) {
  if (objects.length < 2) return;
+  const { x: startX, y: startY } = anchorTopLeft(objects);
  // Normalize widths first (uniform column)
  normalizeWidth(objects);
  // Then arrange as column with even spacing
  const gap = 20;
  const sorted = [...objects].sort((a, b) => a.data.y - b.data.y);
-  const startX = sorted[0].data.x;
-  let y = sorted[0].data.y;
+  let y = 0;
  sorted.forEach((item) => {
    item.data.x = startX;
-    item.data.y = y;
+    item.data.y = startY + y;
    syncPosition(item);
    y += scaledH(item) + gap;
  });
@@ -223,15 +223,22 @@ export function normalizeWidth(objects: SceneItem[]) {

 // ─── Arrangement ───

+/** Get the top-left corner of the bounding box of all items. */
+function anchorTopLeft(objects: SceneItem[]): { x: number; y: number } {
+  return {
+    x: Math.min(...objects.map((item) => item.data.x)),
+    y: Math.min(...objects.map((item) => item.data.y)),
+  };
+}
+
 export function arrangeOptimal(objects: SceneItem[]) {
  if (objects.length < 2) return;
+  const { x: startX, y: startY } = anchorTopLeft(objects);
  // Shelf-based bin packing, sorted by height descending
  const sorted = [...objects].sort((a, b) => scaledH(b) - scaledH(a));
  const gap = 10;
  const totalArea = sorted.reduce((s, item) => s + scaledW(item) * scaledH(item), 0);
  const shelfWidth = Math.sqrt(totalArea) * 1.3;
-  const startX = sorted[0].data.x;
-  const startY = sorted[0].data.y;
  let x = 0, y = 0, shelfHeight = 0;
  sorted.forEach((item) => {
    const w = scaledW(item);
@@ -251,10 +258,9 @@ export function arrangeOptimal(objects: SceneItem[]) {

 export function arrangeGrid(objects: SceneItem[]) {
  if (objects.length < 2) return;
+  const { x: startX, y: startY } = anchorTopLeft(objects);
  const gap = 20;
  const cols = Math.ceil(Math.sqrt(objects.length));
-  const startX = objects[0].data.x;
-  const startY = objects[0].data.y;
  const maxW = Math.max(...objects.map(scaledW));
  const maxH = Math.max(...objects.map(scaledH));
  objects.forEach((item, i) => {
@@ -268,12 +274,12 @@ export function arrangeGrid(objects: SceneItem[]) {

 export function arrangeRow(objects: SceneItem[]) {
  if (objects.length < 2) return;
-  const gap = 20;
+  const { x: startX, y: startY } = anchorTopLeft(objects);
  const sorted = [...objects].sort((a, b) => a.data.x - b.data.x);
-  const startY = sorted[0].data.y;
-  let x = sorted[0].data.x;
+  const gap = 20;
+  let x = 0;
  sorted.forEach((item) => {
-    item.data.x = x;
+    item.data.x = startX + x;
    item.data.y = startY;
    syncPosition(item);
    x += scaledW(item) + gap;
@@ -282,13 +288,13 @@ export function arrangeRow(objects: SceneItem[]) {

 export function arrangeColumn(objects: SceneItem[]) {
  if (objects.length < 2) return;
-  const gap = 20;
+  const { x: startX, y: startY } = anchorTopLeft(objects);
  const sorted = [...objects].sort((a, b) => a.data.y - b.data.y);
-  const startX = sorted[0].data.x;
-  let y = sorted[0].data.y;
+  const gap = 20;
+  let y = 0;
  sorted.forEach((item) => {
    item.data.x = startX;
-    item.data.y = y;
+    item.data.y = startY + y;
    syncPosition(item);
    y += scaledH(item) + gap;
  });
@@ -309,41 +315,40 @@ export function arrangeByName(objects: SceneItem[]) {
  const sorted = [...objects].sort((a, b) =>
    (a.data.name || '').localeCompare(b.data.name || '')
  );
-  layoutAsGrid(sorted);
+  layoutAsGrid(sorted, anchorTopLeft(objects));
 }

 export function arrangeByZOrder(objects: SceneItem[]) {
-  // Sort by z-order stored in item.data.z
  const sorted = [...objects].sort((a, b) => a.data.z - b.data.z);
-  layoutAsGrid(sorted);
+  layoutAsGrid(sorted, anchorTopLeft(objects));
 }

 export function arrangeRandomly(objects: SceneItem[]) {
  if (objects.length < 2) return;
-  const minX = Math.min(...objects.map((item) => item.data.x));
-  const minY = Math.min(...objects.map((item) => item.data.y));
-  const maxX = Math.max(...objects.map((item) => item.data.x + scaledW(item)));
-  const maxY = Math.max(...objects.map((item) => item.data.y + scaledH(item)));
+  const { x: startX, y: startY } = anchorTopLeft(objects);
+  // Compute spread area based on total content size
+  const totalW = objects.reduce((s, item) => s + scaledW(item), 0);
+  const totalH = objects.reduce((s, item) => s + scaledH(item), 0);
+  const spreadW = Math.sqrt(totalW * totalH) * 1.5;
+  const spreadH = spreadW;
  objects.forEach((item) => {
-    item.data.x = minX + Math.random() * (maxX - minX - scaledW(item));
-    item.data.y = minY + Math.random() * (maxY - minY - scaledH(item));
+    item.data.x = startX + Math.random() * spreadW;
+    item.data.y = startY + Math.random() * spreadH;
    syncPosition(item);
  });
 }

-function layoutAsGrid(sorted: SceneItem[]) {
+function layoutAsGrid(sorted: SceneItem[], anchor: { x: number; y: number }) {
  if (sorted.length < 2) return;
  const gap = 20;
  const cols = Math.ceil(Math.sqrt(sorted.length));
-  const startX = sorted[0].data.x;
-  const startY = sorted[0].data.y;
  const maxW = Math.max(...sorted.map(scaledW));
  const maxH = Math.max(...sorted.map(scaledH));
  sorted.forEach((item, i) => {
    const col = i % cols;
    const row = Math.floor(i / cols);
-    item.data.x = startX + col * (maxW + gap);
-    item.data.y = startY + row * (maxH + gap);
+    item.data.x = anchor.x + col * (maxW + gap);
+    item.data.y = anchor.y + row * (maxH + gap);
    syncPosition(item);
  });
 }
@@ -17,6 +17,7 @@ import { ShortcutDef, ShortcutContext } from './shortcuts';
 import type { SceneItem, SceneManager } from './SceneManager';
 import type { GroupObject } from './scene-format';
 import * as ops from './operations';
+import { onArrangeAnimationDone } from './operations';

 // Tracks when the last internal copy happened so paste can decide
 // whether to use internal clipboard (just copied) vs system clipboard (external app).
@@ -121,7 +122,6 @@ function _opUpdate(ctx: ShortcutContext, op: (items: SceneItem[]) => void): void
 /** Like _opUpdate but defers transformBox update until animation completes */
 function _opUpdateAnimated(ctx: ShortcutContext, op: (items: SceneItem[]) => void): void {
  const items = ctx.selection.getSelectedItems();
-  const { onArrangeAnimationDone } = require('./operations');
  onArrangeAnimationDone(() => ctx.selection.transformBox.update(items));
  op(items);
  ctx.onChange(items.map(i => i.id));
@@ -259,13 +259,24 @@ export function setupSync(
  socket.on('element:remove', onElementRemove);
  socket.on('object:transform', onTransformReceived);

-  // ---- Join room ------------------------------------------------------------
+  // ---- Join room (and rejoin on reconnect) ----------------------------------

-  socket.emit('board:join', { boardId }, (response: any) => {
-    if (response?.users) {
-      socket.emit('room:users', { users: response.users });
-    }
-  });
+  function joinRoom() {
+    socket.emit('board:join', { boardId }, (response: any) => {
+      if (response?.users) {
+        socket.emit('room:users', { users: response.users });
+      }
+    });
+  }
+
+  // 'connect' fires on both the initial connection and every reconnect,
+  // ensuring the server always has this client in the board room.
+  socket.on('connect', joinRoom);
+
+  // Emit immediately if already connected (socket was connected before setupSync ran).
+  if (socket.connected) {
+    joinRoom();
+  }

  // ---- Return handle --------------------------------------------------------

@@ -278,6 +289,7 @@ export function setupSync(
    },
    cleanup: () => {
      sceneManager.onChange = prevOnChange;
+      socket.off('connect', joinRoom);
      socket.off('scene:update', onSceneReceived);
      socket.off('element:update', onElementUpdate);
      socket.off('element:remove', onElementRemove);
@@ -1,6 +1,6 @@
 import React from 'react';

-export type SaveStatus = 'saved' | 'saving' | 'unsaved';
+export type SaveStatus = 'saved' | 'saving' | 'unsaved' | 'readonly';

 interface StatusBarProps {
  boardName: string;
@@ -47,10 +47,11 @@ const statusConfig: Record<SaveStatus, { label: string; color: string }> = {
  saved: { label: 'Saved', color: '#69db7c' },
  saving: { label: 'Saving...', color: '#ffd43b' },
  unsaved: { label: 'Unsaved changes', color: '#ff6b6b' },
+  readonly: { label: 'Read-only', color: '#4dabf7' },
 };

 export default function StatusBar({ boardName, imageCount, saveStatus }: StatusBarProps) {
-  const status = statusConfig[saveStatus];
+  const status = statusConfig[saveStatus] || statusConfig.saved;

  return (
    <div style={styles.bar}>
@@ -13,6 +13,7 @@ import { AnnotationStore } from '../stores/annotationStore';
 import { PinOverlay } from '../canvas/PinOverlay';
 // PresenceOverlay removed — remote selection highlighting was too heavy for minimal benefit
 import { connectSocket, disconnectSocket } from '../socket';
+import api from '../api';

 interface OnlineUser {
  userId: string;
@@ -73,10 +74,20 @@ export function useCanvasSetup(deps: CanvasSetupDeps) {
  useEffect(() => {
    if (!boardData || !resolvedBoardId) return;

-    const timer = setTimeout(() => {
+    let attempts = 0;
+    const maxAttempts = 100; // 5 seconds max (100 × 50ms)
+    const poll = setInterval(() => {
+      attempts++;
      const scene = canvasRef.current?.getScene();
      const viewport = canvasRef.current?.getViewport();
-      if (!scene || !viewport) return;
+      if (!scene || !viewport) {
+        if (attempts >= maxAttempts) {
+          clearInterval(poll);
+          console.error('[canvas-setup] Canvas not ready after 5s, giving up');
+        }
+        return;
+      }
+      clearInterval(poll);

      // Create SelectionManager
      const selection = new SelectionManager(viewport, scene);
@@ -309,17 +320,11 @@ export function useCanvasSetup(deps: CanvasSetupDeps) {
        });

        // ── Annotations: load threads, wire socket events ──
-        const token = localStorage.getItem('refboard_token');
-        if (token) {
-          fetch(`/api/boards/${resolvedBoardId}/threads`, {
-            headers: { Authorization: `Bearer ${token}` },
+        api.get(`/api/boards/${resolvedBoardId}/threads`)
+          .then((res) => {
+            if (res.data.threads) annotationStoreRef.current?.loadThreads(res.data.threads);
          })
-            .then((r) => r.json())
-            .then((data) => {
-              if (data.threads) annotationStoreRef.current?.loadThreads(data.threads);
-            })
-            .catch((err) => console.error('[annotations] load threads error:', err));
-        }
+          .catch((err) => console.error('[annotations] load threads error:', err));

        socket.on('thread:add', (data: any) => {
          annotationStoreRef.current?.onThreadAdd(data.thread, data.comment);
@@ -372,10 +377,10 @@ export function useCanvasSetup(deps: CanvasSetupDeps) {
        }
        pasteCleanupRef.current = setupPaste(viewport, scene, resolvedBoardId, onCanvasChange, selection, uploadManager);
      }
-    }, 200);
+    }, 50);

    return () => {
-      clearTimeout(timer);
+      clearInterval(poll);
      selectionRef.current?.destroy();
      selectionRef.current = null;
      if (inboxZoneRef.current) {
@@ -6,6 +6,7 @@ import type { SaveStatus } from '../components/StatusBar';
 interface SaveManagerOptions {
  resolvedBoardId: string | undefined;
  isPublicView?: boolean;
+  readOnly?: boolean;
  canvasRef: React.RefObject<PixiCanvasHandle | null>;
  setSaveStatus: (s: SaveStatus) => void;
 }
@@ -13,11 +14,11 @@ interface SaveManagerOptions {
 /**
 * Debounced save with thumbnail generation from PixiJS renderer.
 */
-export function useSaveManager({ resolvedBoardId, isPublicView, canvasRef, setSaveStatus }: SaveManagerOptions) {
+export function useSaveManager({ resolvedBoardId, isPublicView, readOnly, canvasRef, setSaveStatus }: SaveManagerOptions) {
  const saveTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);

  const scheduleSave = useCallback(() => {
-    if (!resolvedBoardId || isPublicView) return;
+    if (!resolvedBoardId || isPublicView || readOnly) return;
    setSaveStatus('unsaved');
    if (saveTimerRef.current) clearTimeout(saveTimerRef.current);
    saveTimerRef.current = setTimeout(async () => {
@@ -59,11 +60,15 @@ export function useSaveManager({ resolvedBoardId, isPublicView, canvasRef, setSa
        }
        await saveCanvas(resolvedBoardId, state, thumbnail);
        setSaveStatus('saved');
-      } catch {
-        setSaveStatus('unsaved');
+      } catch (err: any) {
+        if (err?.response?.status === 403) {
+          setSaveStatus('readonly');
+        } else {
+          setSaveStatus('unsaved');
+        }
      }
    }, 2000);
-  }, [resolvedBoardId, isPublicView, canvasRef, setSaveStatus]);
+  }, [resolvedBoardId, isPublicView, readOnly, canvasRef, setSaveStatus]);

  return { scheduleSave, saveTimerRef };
 }
@@ -106,6 +106,15 @@ export default function Editor({ isPublicView }: EditorProps) {
  // Derived
  const { boardData, loading, error } = useBoardLoader(boardId);
  const resolvedBoardId = boardId || boardData?.board?.id;
+  const userRole = boardData?.role || 'viewer';
+  const readOnly = !isPublicView && userRole === 'viewer';
+
+  // Set read-only status when board data loads
+  useEffect(() => {
+    if (boardData && readOnly) {
+      setSaveStatus('readonly');
+    }
+  }, [boardData, readOnly]);

  // Toast helper
  const showToast = useCallback((text: string) => {
@@ -115,7 +124,7 @@ export default function Editor({ isPublicView }: EditorProps) {
  }, []);

  // Save manager
-  const { scheduleSave } = useSaveManager({ resolvedBoardId, isPublicView, canvasRef, setSaveStatus });
+  const { scheduleSave } = useSaveManager({ resolvedBoardId, isPublicView, readOnly, canvasRef, setSaveStatus });

  // Canvas change handler.
  // Pass changedIds for incremental sync (fast, lightweight).
@@ -303,18 +312,26 @@ export default function Editor({ isPublicView }: EditorProps) {
    });
  }, [writeCanvasToClipboard, onCanvasChange, handleGroup, handleUngroup, refreshLayers]);

-  // Save on page unload
+  // Save on page unload (only for users with edit access)
  useEffect(() => {
+    if (readOnly || isPublicView) return;
    function onBeforeUnload() {
      const scene = canvasRef.current?.getScene();
      if (!scene || !resolvedBoardId) return;
+      const token = localStorage.getItem('refboard_token');
+      if (!token) return;
      const state = JSON.stringify(scene.serialize());
-      const blob = new Blob([JSON.stringify({ canvas_state: state })], { type: 'application/json' });
-      navigator.sendBeacon(`/api/boards/${resolvedBoardId}/save`, blob);
+      // Use fetch with keepalive to include auth header (sendBeacon can't set headers)
+      fetch(`/api/boards/${resolvedBoardId}/save`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json', 'Authorization': `Bearer ${token}` },
+        body: JSON.stringify({ canvas_state: state }),
+        keepalive: true,
+      }).catch(() => {});
    }
    window.addEventListener('beforeunload', onBeforeUnload);
    return () => window.removeEventListener('beforeunload', onBeforeUnload);
-  }, [resolvedBoardId]);
+  }, [resolvedBoardId, readOnly, isPublicView]);

  // Update UI overlays (selection toolbar, video controls, minimap) on demand
  const updateOverlays = useCallback(() => {
@@ -782,12 +799,13 @@ export default function Editor({ isPublicView }: EditorProps) {
 }

 function StatusIndicator({ status }: { status: SaveStatus }) {
-  const cfg = {
+  const cfg: Record<SaveStatus, { color: string; label: string }> = {
    saved: { color: '#4ade80', label: 'Saved' },
    saving: { color: '#facc15', label: 'Saving...' },
    unsaved: { color: '#f87171', label: 'Unsaved' },
+    readonly: { color: '#4dabf7', label: 'Read-only' },
  };
-  const s = cfg[status];
+  const s = cfg[status] || cfg.saved;
  return (
    <div style={{ display: 'flex', alignItems: 'center', gap: '4px' }}>
      <div style={{ width: '5px', height: '5px', borderRadius: '50%', background: s.color }} />