fix(security): gate saved-dashboard CRUD on connection ACL (#54)

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

@@ -114,6 +114,7 @@ public ResponseEntity<Map<String, Object>> disableShare(@PathVariable UUID id) {
     public ResponseEntity<Map<String, Object>> createDashboard(@RequestBody SavedDashboard savedDashboard) {
         try {
             log.info("Creating saved dashboard: {} for connection: {}", savedDashboard.getName(), savedDashboard.getConnectionId());
+            accessControlService.assertCanManageConnectionContent(savedDashboard.getConnectionId());
 
             SavedDashboard created = savedDashboardService.saveDashboard(savedDashboard);
 
@@ -141,6 +142,7 @@ public ResponseEntity<Map<String, Object>> createDashboard(@RequestBody SavedDas
     public ResponseEntity<Map<String, Object>> getDashboardsByConnection(@PathVariable String connectionId) {
         try {
             log.info("Fetching saved dashboards for connection: {}", connectionId);
+            accessControlService.assertCanReadConnectionContent(connectionId);
 
             List<SavedDashboard> dashboards = savedDashboardService.getDashboardsByConnection(connectionId);
 
@@ -171,6 +173,7 @@ public ResponseEntity<Map<String, Object>> getDashboardById(@PathVariable UUID i
 
             return savedDashboardService.getDashboardById(id)
                     .map(dashboard -> {
+                        accessControlService.assertCanReadConnectionContent(dashboard.getConnectionId());
                         Map<String, Object> response = new HashMap<>();
                         response.put("success", true);
                         response.put("savedDashboard", dashboard);
@@ -200,6 +203,11 @@ public ResponseEntity<Map<String, Object>> getDashboardById(@PathVariable UUID i
     public ResponseEntity<Map<String, Object>> updateDashboard(@PathVariable UUID id, @RequestBody SavedDashboard updates) {
         try {
             log.info("Updating saved dashboard: {}", id);
+            SavedDashboard existing = savedDashboardService.getDashboardById(id)
+                    .orElseThrow(() -> new IllegalArgumentException("Dashboard not found: " + id));
+            // Authorize against the persisted connection — never trust a body
+            // connectionId that could re-attach the row to a different connection.
+            accessControlService.assertCanManageConnectionContent(existing.getConnectionId());
 
             SavedDashboard updated = savedDashboardService.updateDashboard(id, updates);
 
@@ -312,6 +320,9 @@ public ResponseEntity<Map<String, Object>> restoreVersion(@PathVariable UUID id,
     public ResponseEntity<Map<String, Object>> deleteDashboard(@PathVariable UUID id) {
         try {
             log.info("Deleting saved dashboard: {}", id);
+            SavedDashboard existing = savedDashboardService.getDashboardById(id)
+                    .orElseThrow(() -> new IllegalArgumentException("Dashboard not found: " + id));
+            accessControlService.assertCanManageConnectionContent(existing.getConnectionId());
 
             savedDashboardService.deleteDashboard(id);
 
@@ -320,6 +331,12 @@ public ResponseEntity<Map<String, Object>> deleteDashboard(@PathVariable UUID id
             response.put("message", "Dashboard deleted successfully");
 
             return ResponseEntity.ok(response);
+        } catch (IllegalArgumentException e) {
+            log.error("Dashboard not found: {}", id);
+            Map<String, Object> errorResponse = new HashMap<>();
+            errorResponse.put("success", false);
+            errorResponse.put("message", e.getMessage());
+            return ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResponse);
         } catch (org.springframework.web.server.ResponseStatusException e) {
             throw e;
         } catch (Exception e) {
@@ -338,6 +355,9 @@ 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);
+            SavedDashboard existing = savedDashboardService.getDashboardById(id)
+                    .orElseThrow(() -> new IllegalArgumentException("Dashboard not found: " + id));
+            accessControlService.assertCanManageConnectionContent(existing.getConnectionId());
 
             SavedDashboard updated = savedDashboardService.toggleFavorite(id);
 
@@ -373,6 +393,7 @@ public ResponseEntity<Map<String, Object>> toggleFavorite(@PathVariable UUID id)
     public ResponseEntity<Map<String, Object>> getFavoriteDashboards(@PathVariable String connectionId) {
         try {
             log.info("Fetching favorite dashboards for connection: {}", connectionId);
+            accessControlService.assertCanReadConnectionContent(connectionId);
 
             List<SavedDashboard> dashboards = savedDashboardService.getFavoriteDashboards(connectionId);
 
@@ -402,6 +423,7 @@ public ResponseEntity<Map<String, Object>> getDashboardsByFolder(
             @PathVariable String folder) {
         try {
             log.info("Fetching dashboards in folder: {} for connection: {}", folder, connectionId);
+            accessControlService.assertCanReadConnectionContent(connectionId);
 
             List<SavedDashboard> dashboards = savedDashboardService.getDashboardsByFolder(connectionId, folder);
 
@@ -431,6 +453,7 @@ public ResponseEntity<Map<String, Object>> searchDashboards(
             @RequestParam String q) {
         try {
             log.info("Searching dashboards for connection: {} with term: {}", connectionId, q);
+            accessControlService.assertCanReadConnectionContent(connectionId);
 
             List<SavedDashboard> dashboards = savedDashboardService.searchDashboards(connectionId, q);
 
@@ -458,6 +481,7 @@ public ResponseEntity<Map<String, Object>> searchDashboards(
     public ResponseEntity<Map<String, Object>> getFolders(@PathVariable String connectionId) {
         try {
             log.info("Fetching folders for connection: {}", connectionId);
+            accessControlService.assertCanReadConnectionContent(connectionId);
 
             List<String> folders = savedDashboardService.getFolders(connectionId);
 

backend/src/test/java/com/dbaagent/controller/SavedDashboardControllerAccessTest.java

@@ -0,0 +1,217 @@
+package com.dbaagent.controller;
+
+import com.dbaagent.model.SavedDashboard;
+import com.dbaagent.service.SavedDashboardService;
+import com.dbaagent.service.security.AccessControlService;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.server.ResponseStatusException;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.UUID;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Issue #54: create/update/delete (and GETs) must check connection ACL.
+ * Write paths authorize against the persisted connectionId, never a body
+ * connectionId that could re-home a dashboard onto another connection.
+ */
+@ExtendWith(MockitoExtension.class)
+class SavedDashboardControllerAccessTest {
+
+    @Mock private SavedDashboardService savedDashboardService;
+    @Mock private AccessControlService accessControlService;
+
+    private SavedDashboardController controller;
+
+    @BeforeEach
+    void setUp() {
+        controller = new SavedDashboardController();
+        // Field injection mirrors production @Autowired wiring.
+        setField(controller, "savedDashboardService", savedDashboardService);
+        setField(controller, "accessControlService", accessControlService);
+    }
+
+    @Test
+    void create_assertsManageOnBodyConnectionId() {
+        SavedDashboard incoming = dashboard("conn-allowed", "Ops");
+        SavedDashboard saved = dashboard("conn-allowed", "Ops");
+        saved.setId(UUID.randomUUID());
+        when(savedDashboardService.saveDashboard(incoming)).thenReturn(saved);
+
+        ResponseEntity<Map<String, Object>> response = controller.createDashboard(incoming);
+
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED);
+        verify(accessControlService).assertCanManageConnectionContent("conn-allowed");
+        verify(savedDashboardService).saveDashboard(incoming);
+    }
+
+    @Test
+    void create_denied_neverPersists() {
+        SavedDashboard incoming = dashboard("conn-denied", "Leak");
+        doThrow(new ResponseStatusException(HttpStatus.FORBIDDEN, "Content access denied for this connection"))
+            .when(accessControlService).assertCanManageConnectionContent("conn-denied");
+
+        assertThatThrownBy(() -> controller.createDashboard(incoming))
+            .isInstanceOf(ResponseStatusException.class)
+            .extracting(ex -> ((ResponseStatusException) ex).getStatusCode())
+            .isEqualTo(HttpStatus.FORBIDDEN);
+
+        verify(savedDashboardService, never()).saveDashboard(any());
+    }
+
+    @Test
+    void update_assertsManageOnPersistedConnection_notBody() {
+        UUID id = UUID.randomUUID();
+        SavedDashboard existing = dashboard("conn-real", "Existing");
+        existing.setId(id);
+        SavedDashboard body = dashboard("conn-spoofed", "Hijack");
+        SavedDashboard updated = dashboard("conn-real", "Existing");
+        updated.setId(id);
+
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
+        when(savedDashboardService.updateDashboard(id, body)).thenReturn(updated);
+
+        ResponseEntity<Map<String, Object>> response = controller.updateDashboard(id, body);
+
+        assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
+        verify(accessControlService).assertCanManageConnectionContent("conn-real");
+        verify(accessControlService, never()).assertCanManageConnectionContent("conn-spoofed");
+        verify(savedDashboardService).updateDashboard(id, body);
+    }
+
+    @Test
+    void update_denied_neverMutates() {
+        UUID id = UUID.randomUUID();
+        SavedDashboard existing = dashboard("conn-real", "Existing");
+        existing.setId(id);
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
+        doThrow(new ResponseStatusException(HttpStatus.FORBIDDEN, "Content access denied for this connection"))
+            .when(accessControlService).assertCanManageConnectionContent("conn-real");
+
+        assertThatThrownBy(() -> controller.updateDashboard(id, dashboard("conn-spoofed", "x")))
+            .isInstanceOf(ResponseStatusException.class)
+            .extracting(ex -> ((ResponseStatusException) ex).getStatusCode())
+            .isEqualTo(HttpStatus.FORBIDDEN);
+
+        verify(savedDashboardService, never()).updateDashboard(any(), any());
+    }
+
+    @Test
+    void delete_assertsManageOnPersistedConnection() {
+        UUID id = UUID.randomUUID();
+        SavedDashboard existing = dashboard("conn-real", "Existing");
+        existing.setId(id);
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
+
+        ResponseEntity<Map<String, Object>> response = controller.deleteDashboard(id);
+
+        assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
+        verify(accessControlService).assertCanManageConnectionContent("conn-real");
+        verify(savedDashboardService).deleteDashboard(id);
+    }
+
+    @Test
+    void delete_denied_neverDeletes() {
+        UUID id = UUID.randomUUID();
+        SavedDashboard existing = dashboard("conn-real", "Existing");
+        existing.setId(id);
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
+        doThrow(new ResponseStatusException(HttpStatus.FORBIDDEN, "Content access denied for this connection"))
+            .when(accessControlService).assertCanManageConnectionContent("conn-real");
+
+        assertThatThrownBy(() -> controller.deleteDashboard(id))
+            .isInstanceOf(ResponseStatusException.class)
+            .extracting(ex -> ((ResponseStatusException) ex).getStatusCode())
+            .isEqualTo(HttpStatus.FORBIDDEN);
+
+        verify(savedDashboardService, never()).deleteDashboard(any());
+    }
+
+    @Test
+    void delete_missing_returns404() {
+        UUID id = UUID.randomUUID();
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.empty());
+
+        ResponseEntity<Map<String, Object>> response = controller.deleteDashboard(id);
+
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
+        verify(accessControlService, never()).assertCanManageConnectionContent(any());
+        verify(savedDashboardService, never()).deleteDashboard(any());
+    }
+
+    @Test
+    void favorite_assertsManageOnPersistedConnection() {
+        UUID id = UUID.randomUUID();
+        SavedDashboard existing = dashboard("conn-real", "Existing");
+        existing.setId(id);
+        SavedDashboard toggled = dashboard("conn-real", "Existing");
+        toggled.setId(id);
+        toggled.setIsFavorite(true);
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
+        when(savedDashboardService.toggleFavorite(id)).thenReturn(toggled);
+
+        ResponseEntity<Map<String, Object>> response = controller.toggleFavorite(id);
+
+        assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
+        verify(accessControlService).assertCanManageConnectionContent("conn-real");
+        verify(savedDashboardService).toggleFavorite(id);
+    }
+
+    @Test
+    void getById_assertsReadOnPersistedConnection() {
+        UUID id = UUID.randomUUID();
+        SavedDashboard existing = dashboard("conn-real", "Existing");
+        existing.setId(id);
+        when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
+
+        ResponseEntity<Map<String, Object>> response = controller.getDashboardById(id);
+
+        assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
+        verify(accessControlService).assertCanReadConnectionContent("conn-real");
+    }
+
+    @Test
+    void listByConnection_assertsRead() {
+        when(savedDashboardService.getDashboardsByConnection("conn-1")).thenReturn(List.of());
+
+        ResponseEntity<Map<String, Object>> response = controller.getDashboardsByConnection("conn-1");
+
+        assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
+        verify(accessControlService).assertCanReadConnectionContent("conn-1");
+    }
+
+    private static SavedDashboard dashboard(String connectionId, String name) {
+        SavedDashboard d = new SavedDashboard();
+        d.setConnectionId(connectionId);
+        d.setName(name);
+        d.setDashboardConfig("{}");
+        d.setIsFavorite(false);
+        d.setIsPublic(false);
+        return d;
+    }
+
+    private static void setField(Object target, String name, Object value) {
+        try {
+            var field = SavedDashboardController.class.getDeclaredField(name);
+            field.setAccessible(true);
+            field.set(target, value);
+        } catch (ReflectiveOperationException e) {
+            throw new IllegalStateException(e);
+        }
+    }
+}