fix(dashboards): close favorite-toggle IDOR and tighten workspace guards

Addresses PR #80 review.

BLOCKER — POST /saved-dashboards/{id}/favorite had no authorization at all. A
non-member could toggle the favorite flag on a workspace-restricted dashboard
and, worse than reported, read the entire row back from the 200 response —
dashboardConfig and chatMessages included — bypassing the very 404 that hides
it. Reproduced live before the fix (200, is_favorite f->t, full config in the
body); now 404 with the row untouched and nothing leaked.

An audit of every handler in the controller found this was the only one missing
its gate: the list endpoints use filterReadable, and the rest already assert
both connection access and workspace membership.

Also fixes the review's follow-ups:

- assertWorkspaceAssignable called getWorkspace(), which asserts only
  visibility, so a VIEWER could create dashboards into a workspace while
  moveDashboard() required MANAGER for the same effect. Added
  assertCanAssignInto() so both paths agree, plus a connection-match check.
- MANAGE_DASHBOARD_WORKSPACES was declared in the Permission enum and offered in
  the role editor but enforced nowhere, so uticking it had no effect — worse
  than not offering the toggle. createWorkspace now asserts it.
- getFolders ran a DISTINCT over every dashboard on the connection, leaking the
  folder names of workspace-restricted ones. It now derives folders from the
  readable set (verified: non-member sees ['hi'], admin sees
  ['SecretFolder','hi']).

Note on how this was missed: the earlier QA exercised the favorite toggle only
as admin, which passes regardless of the guard. Testing a permission check
requires the role that should fail.

Author: notSumit25

backend/src/main/java/com/dbaagent/controller/SavedDashboardController.java

@@ -1,6 +1,7 @@
 package com.dbaagent.controller;
 
 import com.dbaagent.model.DashboardVersion;
+import com.dbaagent.model.DashboardWorkspace;
 import com.dbaagent.model.SavedDashboard;
 import com.dbaagent.service.ConnectionChatAccessPolicyService;
 import com.dbaagent.service.DashboardWorkspaceService;
@@ -12,6 +13,7 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
+import org.springframework.web.server.ResponseStatusException;
 
 import java.util.HashMap;
 import java.util.List;
@@ -49,14 +51,23 @@ private static ResponseEntity<Map<String, Object>> conflict(OptimisticLockingFai
     }
 
     /**
-     * A dashboard may only be created into a workspace the caller can actually manage —
-     * otherwise anyone could push a dashboard into someone else's workspace.
+     * A dashboard may only be created into a workspace the caller can actually manage.
+     *
+     * <p>This previously called {@code getWorkspace}, which asserts only *visibility* —
+     * so a VIEWER could create dashboards into a workspace they merely belonged to,
+     * despite {@code moveDashboard} requiring MANAGER for the same effect. The two paths
+     * now agree.
      */
     private void assertWorkspaceAssignable(String connectionId, UUID workspaceId) {
         if (workspaceId == null) {
             return;
         }
-        dashboardWorkspaceService.getWorkspace(workspaceId);
+        DashboardWorkspace workspace = dashboardWorkspaceService.getWorkspace(workspaceId);
+        if (!workspace.getConnectionId().equals(connectionId)) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                "Workspace belongs to a different connection");
+        }
+        dashboardWorkspaceService.assertCanAssignInto(workspaceId);
     }
 
     /** Publish this dashboard to the web (opt-in, revocable public link). */
@@ -384,6 +395,15 @@ public ResponseEntity<Map<String, Object>> deleteDashboard(@PathVariable UUID id
     public ResponseEntity<Map<String, Object>> toggleFavorite(@PathVariable UUID id) {
         try {
             log.info("Toggling favorite for dashboard: {}", id);
+            // This handler had NO authorization at all, so a non-member could toggle the
+            // favorite flag on a workspace-restricted dashboard and — worse — read the
+            // whole row back from the 200 response (dashboardConfig and chatMessages
+            // included), bypassing the very 404 that hides it. Load first, then apply the
+            // same connection + workspace gate every other handler here uses.
+            SavedDashboard existing = savedDashboardService.getDashboardById(id)
+                .orElseThrow(() -> new IllegalArgumentException("Dashboard not found"));
+            accessControlService.assertCanReadConnectionContent(existing.getConnectionId());
+            dashboardWorkspaceService.assertCanReadDashboard(existing);
 
             SavedDashboard updated = savedDashboardService.toggleFavorite(id);
 
@@ -512,7 +532,18 @@ public ResponseEntity<Map<String, Object>> getFolders(@PathVariable String conne
             log.info("Fetching folders for connection: {}", connectionId);
             accessControlService.assertCanReadConnectionContent(connectionId);
 
-            List<String> folders = savedDashboardService.getFolders(connectionId);
+            // Derive folders from the dashboards this caller can actually see. The
+            // repository query is a DISTINCT over every dashboard on the connection, so a
+            // non-member learned the folder names of workspace-restricted dashboards —
+            // a small leak, but through the same list the 404s are meant to hide.
+            List<String> folders = dashboardWorkspaceService
+                .filterReadable(savedDashboardService.getDashboardsByConnection(connectionId))
+                .stream()
+                .map(SavedDashboard::getFolder)
+                .filter(f -> f != null && !f.isBlank())
+                .distinct()
+                .sorted()
+                .toList();
 
             Map<String, Object> response = new HashMap<>();
             response.put("success", true);

backend/src/main/java/com/dbaagent/service/DashboardWorkspaceService.java

@@ -1,6 +1,7 @@
 package com.dbaagent.service;
 
 import com.dbaagent.model.DashboardWorkspace;
+import com.dbaagent.model.Permission;
 import com.dbaagent.model.DashboardWorkspaceMember;
 import com.dbaagent.model.DashboardWorkspaceRole;
 import com.dbaagent.model.SavedDashboard;
@@ -97,6 +98,11 @@ public List<SavedDashboard> listDashboards(UUID workspaceId) {
     @Transactional
     public DashboardWorkspace createWorkspace(String connectionId, String name, String description, String color) {
         accessControlService.assertCanReadConnectionContent(connectionId);
+        // MANAGE_DASHBOARD_WORKSPACES was declared in the Permission enum and offered in
+        // the role editor, but nothing enforced it — an admin who unticked it saw no
+        // effect, which is worse than not offering the toggle at all.
+        accessControlService.assertHasPermission(Permission.MANAGE_DASHBOARD_WORKSPACES,
+            "You do not have permission to create dashboard workspaces");
         String cleanName = requireName(name);
 
         workspaceRepository.findByConnectionIdAndNameIgnoreCase(connectionId, cleanName).ifPresent(existing -> {
@@ -315,6 +321,23 @@ private void assertCanView(DashboardWorkspace workspace) {
             .orElseThrow(() -> new ResponseStatusException(NOT_FOUND, "Workspace not found"));
     }
 
+    /**
+     * Assert the caller may put a dashboard into this workspace.
+     *
+     * <p>Deliberately manage-level, not view-level: a VIEWER can open a workspace but must
+     * not be able to push new dashboards into someone else's, which is what checking only
+     * visibility allowed. Mirrors {@link #moveDashboard}, so creating into a workspace and
+     * moving into one now require the same thing.
+     */
+    public void assertCanAssignInto(UUID workspaceId) {
+        if (workspaceId == null) {
+            return;
+        }
+        DashboardWorkspace workspace = workspaceRepository.findById(workspaceId)
+            .orElseThrow(() -> new ResponseStatusException(NOT_FOUND, "Workspace not found"));
+        assertCanManage(workspace);
+    }
+
     private void assertCanManage(DashboardWorkspace workspace) {
         accessControlService.assertCanReadConnectionContent(workspace.getConnectionId());
         if (accessControlService.isCurrentUserAdmin()) {

backend/src/main/java/com/dbaagent/service/security/AccessControlService.java

@@ -206,6 +206,16 @@ public void assertCanManageConnections() {
         }
     }
 
+    /** Assert the caller holds a permission, with a caller-supplied message. */
+    public void assertHasPermission(Permission permission, String message) {
+        if (!authEnabled) {
+            return;
+        }
+        if (!hasPermission(permission)) {
+            throw new ResponseStatusException(FORBIDDEN, message);
+        }
+    }
+
     /**
      * Whether the current principal carries a permission authority.
      *

backend/src/test/java/com/dbaagent/service/DashboardWorkspaceAccessTest.java

@@ -193,6 +193,50 @@ void cannotRemoveLastManager() {
             .hasMessageContaining("409");
     }
 
+    @Test
+    @DisplayName("assertCanAssignInto requires MANAGER, not mere membership")
+    void assignIntoRequiresManager() {
+        // Creating a dashboard into a workspace used to call getWorkspace(), which asserts
+        // only visibility — so a VIEWER could push dashboards into a workspace while
+        // moveDashboard() required MANAGER for the same effect.
+        DashboardWorkspace workspace = new DashboardWorkspace();
+        workspace.setId(WORKSPACE);
+        workspace.setConnectionId(CONNECTION);
+        when(workspaceRepository.findById(WORKSPACE)).thenReturn(Optional.of(workspace));
+
+        DashboardWorkspaceMember viewer = new DashboardWorkspaceMember();
+        viewer.setWorkspaceId(WORKSPACE);
+        viewer.setUsername("analyst");
+        viewer.setWorkspaceRole(DashboardWorkspaceRole.VIEWER);
+        when(memberRepository.findByWorkspaceIdAndUsernameIgnoreCase(WORKSPACE, "analyst"))
+            .thenReturn(Optional.of(viewer));
+
+        assertThatThrownBy(() -> service.assertCanAssignInto(WORKSPACE))
+            .isInstanceOf(ResponseStatusException.class)
+            .hasMessageContaining("403");
+
+        // A manager passes.
+        viewer.setWorkspaceRole(DashboardWorkspaceRole.MANAGER);
+        org.junit.jupiter.api.Assertions.assertDoesNotThrow(() -> service.assertCanAssignInto(WORKSPACE));
+    }
+
+    @Test
+    @DisplayName("assertCanReadDashboard is what stops the favorite-toggle IDOR")
+    void favoriteTogglePathIsGated() {
+        // POST /saved-dashboards/{id}/favorite had no authorization at all: a non-member
+        // could flip the flag on a workspace-restricted dashboard AND read the whole row
+        // back from the 200 response, bypassing the 404 that hides it. The controller now
+        // runs this same gate before toggling.
+        when(memberRepository.findByWorkspaceIdAndUsernameIgnoreCase(WORKSPACE, "analyst"))
+            .thenReturn(Optional.empty());
+
+        SavedDashboard restricted = dashboard(WORKSPACE);
+
+        assertThatThrownBy(() -> service.assertCanReadDashboard(restricted))
+            .isInstanceOf(ResponseStatusException.class)
+            .hasMessageContaining("404");
+    }
+
     @Test
     @DisplayName("A dashboard cannot be moved into a workspace on a different connection")
     void cannotMoveAcrossConnections() {