Skip to content

Commit 28e406b

Browse files
cursoragentvenkateshsakamuri-lab
cursoragent
and
venkateshsakamuri-lab
committed
fix(security): gate saved-dashboard CRUD on connection ACL
Mirror SavedQueryController: manage on create/update/delete/favorite (persisted connectionId for mutations), read on GETs. Closes #54. Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
1 parent 1925eca commit 28e406b

2 files changed

Lines changed: 241 additions & 0 deletions

File tree

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ public ResponseEntity<Map<String, Object>> disableShare(@PathVariable UUID id) {
113113
public ResponseEntity<Map<String, Object>> createDashboard(@RequestBody SavedDashboard savedDashboard) {
114114
try {
115115
log.info("Creating saved dashboard: {} for connection: {}", savedDashboard.getName(), savedDashboard.getConnectionId());
116+
accessControlService.assertCanManageConnectionContent(savedDashboard.getConnectionId());
116117

117118
SavedDashboard created = savedDashboardService.saveDashboard(savedDashboard);
118119

@@ -140,6 +141,7 @@ public ResponseEntity<Map<String, Object>> createDashboard(@RequestBody SavedDas
140141
public ResponseEntity<Map<String, Object>> getDashboardsByConnection(@PathVariable String connectionId) {
141142
try {
142143
log.info("Fetching saved dashboards for connection: {}", connectionId);
144+
accessControlService.assertCanReadConnectionContent(connectionId);
143145

144146
List<SavedDashboard> dashboards = savedDashboardService.getDashboardsByConnection(connectionId);
145147

@@ -170,6 +172,7 @@ public ResponseEntity<Map<String, Object>> getDashboardById(@PathVariable UUID i
170172

171173
return savedDashboardService.getDashboardById(id)
172174
.map(dashboard -> {
175+
accessControlService.assertCanReadConnectionContent(dashboard.getConnectionId());
173176
Map<String, Object> response = new HashMap<>();
174177
response.put("success", true);
175178
response.put("savedDashboard", dashboard);
@@ -199,6 +202,11 @@ public ResponseEntity<Map<String, Object>> getDashboardById(@PathVariable UUID i
199202
public ResponseEntity<Map<String, Object>> updateDashboard(@PathVariable UUID id, @RequestBody SavedDashboard updates) {
200203
try {
201204
log.info("Updating saved dashboard: {}", id);
205+
SavedDashboard existing = savedDashboardService.getDashboardById(id)
206+
.orElseThrow(() -> new IllegalArgumentException("Dashboard not found: " + id));
207+
// Authorize against the persisted connection — never trust a body
208+
// connectionId that could re-attach the row to a different connection.
209+
accessControlService.assertCanManageConnectionContent(existing.getConnectionId());
202210

203211
SavedDashboard updated = savedDashboardService.updateDashboard(id, updates);
204212

@@ -234,6 +242,9 @@ public ResponseEntity<Map<String, Object>> updateDashboard(@PathVariable UUID id
234242
public ResponseEntity<Map<String, Object>> deleteDashboard(@PathVariable UUID id) {
235243
try {
236244
log.info("Deleting saved dashboard: {}", id);
245+
SavedDashboard existing = savedDashboardService.getDashboardById(id)
246+
.orElseThrow(() -> new IllegalArgumentException("Dashboard not found: " + id));
247+
accessControlService.assertCanManageConnectionContent(existing.getConnectionId());
237248

238249
savedDashboardService.deleteDashboard(id);
239250

@@ -242,6 +253,12 @@ public ResponseEntity<Map<String, Object>> deleteDashboard(@PathVariable UUID id
242253
response.put("message", "Dashboard deleted successfully");
243254

244255
return ResponseEntity.ok(response);
256+
} catch (IllegalArgumentException e) {
257+
log.error("Dashboard not found: {}", id);
258+
Map<String, Object> errorResponse = new HashMap<>();
259+
errorResponse.put("success", false);
260+
errorResponse.put("message", e.getMessage());
261+
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResponse);
245262
} catch (org.springframework.web.server.ResponseStatusException e) {
246263
throw e;
247264
} catch (Exception e) {
@@ -260,6 +277,9 @@ public ResponseEntity<Map<String, Object>> deleteDashboard(@PathVariable UUID id
260277
public ResponseEntity<Map<String, Object>> toggleFavorite(@PathVariable UUID id) {
261278
try {
262279
log.info("Toggling favorite for dashboard: {}", id);
280+
SavedDashboard existing = savedDashboardService.getDashboardById(id)
281+
.orElseThrow(() -> new IllegalArgumentException("Dashboard not found: " + id));
282+
accessControlService.assertCanManageConnectionContent(existing.getConnectionId());
263283

264284
SavedDashboard updated = savedDashboardService.toggleFavorite(id);
265285

@@ -295,6 +315,7 @@ public ResponseEntity<Map<String, Object>> toggleFavorite(@PathVariable UUID id)
295315
public ResponseEntity<Map<String, Object>> getFavoriteDashboards(@PathVariable String connectionId) {
296316
try {
297317
log.info("Fetching favorite dashboards for connection: {}", connectionId);
318+
accessControlService.assertCanReadConnectionContent(connectionId);
298319

299320
List<SavedDashboard> dashboards = savedDashboardService.getFavoriteDashboards(connectionId);
300321

@@ -324,6 +345,7 @@ public ResponseEntity<Map<String, Object>> getDashboardsByFolder(
324345
@PathVariable String folder) {
325346
try {
326347
log.info("Fetching dashboards in folder: {} for connection: {}", folder, connectionId);
348+
accessControlService.assertCanReadConnectionContent(connectionId);
327349

328350
List<SavedDashboard> dashboards = savedDashboardService.getDashboardsByFolder(connectionId, folder);
329351

@@ -353,6 +375,7 @@ public ResponseEntity<Map<String, Object>> searchDashboards(
353375
@RequestParam String q) {
354376
try {
355377
log.info("Searching dashboards for connection: {} with term: {}", connectionId, q);
378+
accessControlService.assertCanReadConnectionContent(connectionId);
356379

357380
List<SavedDashboard> dashboards = savedDashboardService.searchDashboards(connectionId, q);
358381

@@ -380,6 +403,7 @@ public ResponseEntity<Map<String, Object>> searchDashboards(
380403
public ResponseEntity<Map<String, Object>> getFolders(@PathVariable String connectionId) {
381404
try {
382405
log.info("Fetching folders for connection: {}", connectionId);
406+
accessControlService.assertCanReadConnectionContent(connectionId);
383407

384408
List<String> folders = savedDashboardService.getFolders(connectionId);
385409

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

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
package com.dbaagent.controller;
2+
3+
import com.dbaagent.model.SavedDashboard;
4+
import com.dbaagent.service.SavedDashboardService;
5+
import com.dbaagent.service.security.AccessControlService;
6+
import org.junit.jupiter.api.BeforeEach;
7+
import org.junit.jupiter.api.Test;
8+
import org.junit.jupiter.api.extension.ExtendWith;
9+
import org.mockito.Mock;
10+
import org.mockito.junit.jupiter.MockitoExtension;
11+
import org.springframework.http.HttpStatus;
12+
import org.springframework.http.ResponseEntity;
13+
import org.springframework.web.server.ResponseStatusException;
14+
15+
import java.util.List;
16+
import java.util.Map;
17+
import java.util.Optional;
18+
import java.util.UUID;
19+
20+
import static org.assertj.core.api.Assertions.assertThat;
21+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
22+
import static org.mockito.ArgumentMatchers.any;
23+
import static org.mockito.Mockito.doThrow;
24+
import static org.mockito.Mockito.never;
25+
import static org.mockito.Mockito.verify;
26+
import static org.mockito.Mockito.when;
27+
28+
/**
29+
* Issue #54: create/update/delete (and GETs) must check connection ACL.
30+
* Write paths authorize against the persisted connectionId, never a body
31+
* connectionId that could re-home a dashboard onto another connection.
32+
*/
33+
@ExtendWith(MockitoExtension.class)
34+
class SavedDashboardControllerAccessTest {
35+
36+
@Mock private SavedDashboardService savedDashboardService;
37+
@Mock private AccessControlService accessControlService;
38+
39+
private SavedDashboardController controller;
40+
41+
@BeforeEach
42+
void setUp() {
43+
controller = new SavedDashboardController();
44+
// Field injection mirrors production @Autowired wiring.
45+
setField(controller, "savedDashboardService", savedDashboardService);
46+
setField(controller, "accessControlService", accessControlService);
47+
}
48+
49+
@Test
50+
void create_assertsManageOnBodyConnectionId() {
51+
SavedDashboard incoming = dashboard("conn-allowed", "Ops");
52+
SavedDashboard saved = dashboard("conn-allowed", "Ops");
53+
saved.setId(UUID.randomUUID());
54+
when(savedDashboardService.saveDashboard(incoming)).thenReturn(saved);
55+
56+
ResponseEntity<Map<String, Object>> response = controller.createDashboard(incoming);
57+
58+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED);
59+
verify(accessControlService).assertCanManageConnectionContent("conn-allowed");
60+
verify(savedDashboardService).saveDashboard(incoming);
61+
}
62+
63+
@Test
64+
void create_denied_neverPersists() {
65+
SavedDashboard incoming = dashboard("conn-denied", "Leak");
66+
doThrow(new ResponseStatusException(HttpStatus.FORBIDDEN, "Content access denied for this connection"))
67+
.when(accessControlService).assertCanManageConnectionContent("conn-denied");
68+
69+
assertThatThrownBy(() -> controller.createDashboard(incoming))
70+
.isInstanceOf(ResponseStatusException.class)
71+
.extracting(ex -> ((ResponseStatusException) ex).getStatusCode())
72+
.isEqualTo(HttpStatus.FORBIDDEN);
73+
74+
verify(savedDashboardService, never()).saveDashboard(any());
75+
}
76+
77+
@Test
78+
void update_assertsManageOnPersistedConnection_notBody() {
79+
UUID id = UUID.randomUUID();
80+
SavedDashboard existing = dashboard("conn-real", "Existing");
81+
existing.setId(id);
82+
SavedDashboard body = dashboard("conn-spoofed", "Hijack");
83+
SavedDashboard updated = dashboard("conn-real", "Existing");
84+
updated.setId(id);
85+
86+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
87+
when(savedDashboardService.updateDashboard(id, body)).thenReturn(updated);
88+
89+
ResponseEntity<Map<String, Object>> response = controller.updateDashboard(id, body);
90+
91+
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
92+
verify(accessControlService).assertCanManageConnectionContent("conn-real");
93+
verify(accessControlService, never()).assertCanManageConnectionContent("conn-spoofed");
94+
verify(savedDashboardService).updateDashboard(id, body);
95+
}
96+
97+
@Test
98+
void update_denied_neverMutates() {
99+
UUID id = UUID.randomUUID();
100+
SavedDashboard existing = dashboard("conn-real", "Existing");
101+
existing.setId(id);
102+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
103+
doThrow(new ResponseStatusException(HttpStatus.FORBIDDEN, "Content access denied for this connection"))
104+
.when(accessControlService).assertCanManageConnectionContent("conn-real");
105+
106+
assertThatThrownBy(() -> controller.updateDashboard(id, dashboard("conn-spoofed", "x")))
107+
.isInstanceOf(ResponseStatusException.class)
108+
.extracting(ex -> ((ResponseStatusException) ex).getStatusCode())
109+
.isEqualTo(HttpStatus.FORBIDDEN);
110+
111+
verify(savedDashboardService, never()).updateDashboard(any(), any());
112+
}
113+
114+
@Test
115+
void delete_assertsManageOnPersistedConnection() {
116+
UUID id = UUID.randomUUID();
117+
SavedDashboard existing = dashboard("conn-real", "Existing");
118+
existing.setId(id);
119+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
120+
121+
ResponseEntity<Map<String, Object>> response = controller.deleteDashboard(id);
122+
123+
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
124+
verify(accessControlService).assertCanManageConnectionContent("conn-real");
125+
verify(savedDashboardService).deleteDashboard(id);
126+
}
127+
128+
@Test
129+
void delete_denied_neverDeletes() {
130+
UUID id = UUID.randomUUID();
131+
SavedDashboard existing = dashboard("conn-real", "Existing");
132+
existing.setId(id);
133+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
134+
doThrow(new ResponseStatusException(HttpStatus.FORBIDDEN, "Content access denied for this connection"))
135+
.when(accessControlService).assertCanManageConnectionContent("conn-real");
136+
137+
assertThatThrownBy(() -> controller.deleteDashboard(id))
138+
.isInstanceOf(ResponseStatusException.class)
139+
.extracting(ex -> ((ResponseStatusException) ex).getStatusCode())
140+
.isEqualTo(HttpStatus.FORBIDDEN);
141+
142+
verify(savedDashboardService, never()).deleteDashboard(any());
143+
}
144+
145+
@Test
146+
void delete_missing_returns404() {
147+
UUID id = UUID.randomUUID();
148+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.empty());
149+
150+
ResponseEntity<Map<String, Object>> response = controller.deleteDashboard(id);
151+
152+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
153+
verify(accessControlService, never()).assertCanManageConnectionContent(any());
154+
verify(savedDashboardService, never()).deleteDashboard(any());
155+
}
156+
157+
@Test
158+
void favorite_assertsManageOnPersistedConnection() {
159+
UUID id = UUID.randomUUID();
160+
SavedDashboard existing = dashboard("conn-real", "Existing");
161+
existing.setId(id);
162+
SavedDashboard toggled = dashboard("conn-real", "Existing");
163+
toggled.setId(id);
164+
toggled.setIsFavorite(true);
165+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
166+
when(savedDashboardService.toggleFavorite(id)).thenReturn(toggled);
167+
168+
ResponseEntity<Map<String, Object>> response = controller.toggleFavorite(id);
169+
170+
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
171+
verify(accessControlService).assertCanManageConnectionContent("conn-real");
172+
verify(savedDashboardService).toggleFavorite(id);
173+
}
174+
175+
@Test
176+
void getById_assertsReadOnPersistedConnection() {
177+
UUID id = UUID.randomUUID();
178+
SavedDashboard existing = dashboard("conn-real", "Existing");
179+
existing.setId(id);
180+
when(savedDashboardService.getDashboardById(id)).thenReturn(Optional.of(existing));
181+
182+
ResponseEntity<Map<String, Object>> response = controller.getDashboardById(id);
183+
184+
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
185+
verify(accessControlService).assertCanReadConnectionContent("conn-real");
186+
}
187+
188+
@Test
189+
void listByConnection_assertsRead() {
190+
when(savedDashboardService.getDashboardsByConnection("conn-1")).thenReturn(List.of());
191+
192+
ResponseEntity<Map<String, Object>> response = controller.getDashboardsByConnection("conn-1");
193+
194+
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue();
195+
verify(accessControlService).assertCanReadConnectionContent("conn-1");
196+
}
197+
198+
private static SavedDashboard dashboard(String connectionId, String name) {
199+
SavedDashboard d = new SavedDashboard();
200+
d.setConnectionId(connectionId);
201+
d.setName(name);
202+
d.setDashboardConfig("{}");
203+
d.setIsFavorite(false);
204+
d.setIsPublic(false);
205+
return d;
206+
}
207+
208+
private static void setField(Object target, String name, Object value) {
209+
try {
210+
var field = SavedDashboardController.class.getDeclaredField(name);
211+
field.setAccessible(true);
212+
field.set(target, value);
213+
} catch (ReflectiveOperationException e) {
214+
throw new IllegalStateException(e);
215+
}
216+
}
217+
}

0 commit comments

Comments
 (0)