Skip to content

Commit 73af4c4

Browse files
committed
feat(dashboards): workspaces to group dashboards with per-member access
A DashboardWorkspace groups dashboards within one connection and carries its own member list, keyed by username to match connection_access_grant so "View as" resolves membership as the target user. The access rule is an AND, and it only ever narrows: connection access is checked first and unchanged, and workspace membership is an additional gate on top. Adding someone to a workspace can therefore never grant them a connection they were not already given. saved_dashboards.workspace_id is nullable — NULL means "not grouped", governed purely by the connection ACL exactly as before. Admins bypass the membership half, as they already bypass connection grants. Non-membership reports 404, not 403: a user outside the workspace must not learn the dashboard exists. Deleting a workspace detaches its dashboards rather than cascading — deleting a grouping must never destroy the things grouped — and removing the last MANAGER is refused so a workspace cannot be orphaned. Also closes a pre-existing authorization hole this feature sat on top of: /saved-dashboards create, list, get, update and delete took a caller-supplied connectionId or id and checked nothing, so any authenticated user could read every dashboard on every connection. Verified live before the fix by reading dashboards on a connection the user held no grant on. All of them now assert connection access and the workspace gate; DashboardAlertController does the same through its single requireDashboard choke point.
1 parent c43a26a commit 73af4c4

16 files changed

Lines changed: 2051 additions & 7 deletions

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.dbaagent.model.SavedDashboard;
55
import com.dbaagent.service.DashboardAlertService;
66
import com.dbaagent.service.SavedDashboardService;
7+
import com.dbaagent.service.DashboardWorkspaceService;
78
import com.dbaagent.service.security.AccessControlService;
89
import lombok.RequiredArgsConstructor;
910
import lombok.extern.slf4j.Slf4j;
@@ -25,6 +26,7 @@ public class DashboardAlertController {
2526
private final DashboardAlertService alertService;
2627
private final SavedDashboardService savedDashboardService;
2728
private final AccessControlService accessControlService;
29+
private final DashboardWorkspaceService dashboardWorkspaceService;
2830

2931
@PostMapping
3032
public ResponseEntity<Map<String, Object>> create(@PathVariable UUID dashboardId, @RequestBody DashboardAlert draft) {
@@ -93,8 +95,15 @@ public ResponseEntity<Map<String, Object>> delete(@PathVariable UUID dashboardId
9395
}
9496
}
9597

98+
/**
99+
* The single point every handler here resolves a dashboard through, so the workspace
100+
* membership gate applies to all of them at once. The connection check stays with
101+
* each caller because read and write paths need different assertions.
102+
*/
96103
private SavedDashboard requireDashboard(UUID dashboardId) {
97-
return savedDashboardService.getDashboardById(dashboardId)
104+
SavedDashboard dashboard = savedDashboardService.getDashboardById(dashboardId)
98105
.orElseThrow(() -> new IllegalArgumentException("Dashboard not found"));
106+
dashboardWorkspaceService.assertCanReadDashboard(dashboard);
107+
return dashboard;
99108
}
100109
}
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
package com.dbaagent.controller;
2+
3+
import com.dbaagent.model.DashboardWorkspace;
4+
import com.dbaagent.model.DashboardWorkspaceMember;
5+
import com.dbaagent.model.SavedDashboard;
6+
import com.dbaagent.repository.SavedDashboardRepository;
7+
import com.dbaagent.service.DashboardWorkspaceService;
8+
import lombok.RequiredArgsConstructor;
9+
import lombok.extern.slf4j.Slf4j;
10+
import org.springframework.http.HttpStatus;
11+
import org.springframework.http.ResponseEntity;
12+
import org.springframework.web.bind.annotation.*;
13+
import org.springframework.web.server.ResponseStatusException;
14+
15+
import java.util.*;
16+
17+
/**
18+
* Dashboard workspaces: grouping dashboards with their own member list.
19+
*
20+
* <pre>
21+
* GET /dashboard-workspaces/connection/{connectionId} workspaces I can see
22+
* POST /dashboard-workspaces create
23+
* GET /dashboard-workspaces/{id} one workspace
24+
* PUT /dashboard-workspaces/{id} rename / recolour
25+
* DELETE /dashboard-workspaces/{id} delete (detaches dashboards)
26+
* GET /dashboard-workspaces/{id}/dashboards dashboards inside it
27+
* GET /dashboard-workspaces/{id}/members member list
28+
* POST /dashboard-workspaces/{id}/members add or change a member
29+
* DELETE /dashboard-workspaces/{id}/members/{username} remove a member
30+
* PUT /dashboard-workspaces/dashboards/{dashboardId} move a dashboard in/out
31+
* </pre>
32+
*
33+
* <p>Every method delegates its access check to {@link DashboardWorkspaceService}, which
34+
* asserts connection access first and workspace membership second. As elsewhere in this
35+
* codebase there is no filter doing this for you — a new endpoint here must call the
36+
* service, never the repositories directly.
37+
*/
38+
@RestController
39+
@RequestMapping("/dashboard-workspaces")
40+
@RequiredArgsConstructor
41+
@Slf4j
42+
public class DashboardWorkspaceController {
43+
44+
private final DashboardWorkspaceService workspaceService;
45+
private final SavedDashboardRepository savedDashboardRepository;
46+
47+
@GetMapping("/connection/{connectionId}")
48+
public ResponseEntity<?> listWorkspaces(@PathVariable String connectionId) {
49+
try {
50+
List<DashboardWorkspace> workspaces = workspaceService.listVisibleWorkspaces(connectionId);
51+
return ResponseEntity.ok(workspaces.stream().map(this::describe).toList());
52+
} catch (ResponseStatusException e) {
53+
throw e;
54+
} catch (Exception e) {
55+
log.error("Error listing dashboard workspaces", e);
56+
return failure("Failed to load workspaces");
57+
}
58+
}
59+
60+
@PostMapping
61+
public ResponseEntity<?> createWorkspace(@RequestBody Map<String, Object> body) {
62+
try {
63+
String connectionId = asString(body.get("connectionId"));
64+
if (connectionId == null) {
65+
return ResponseEntity.badRequest().body(Map.of("success", false, "message", "connectionId is required"));
66+
}
67+
DashboardWorkspace workspace = workspaceService.createWorkspace(
68+
connectionId,
69+
asString(body.get("name")),
70+
asString(body.get("description")),
71+
asString(body.get("color"))
72+
);
73+
return ResponseEntity.ok(describe(workspace));
74+
} catch (ResponseStatusException e) {
75+
throw e;
76+
} catch (Exception e) {
77+
log.error("Error creating dashboard workspace", e);
78+
return failure("Failed to create workspace");
79+
}
80+
}
81+
82+
@GetMapping("/{id}")
83+
public ResponseEntity<?> getWorkspace(@PathVariable UUID id) {
84+
try {
85+
return ResponseEntity.ok(describe(workspaceService.getWorkspace(id)));
86+
} catch (ResponseStatusException e) {
87+
throw e;
88+
} catch (Exception e) {
89+
log.error("Error loading dashboard workspace", e);
90+
return failure("Failed to load workspace");
91+
}
92+
}
93+
94+
@PutMapping("/{id}")
95+
public ResponseEntity<?> updateWorkspace(@PathVariable UUID id, @RequestBody Map<String, Object> body) {
96+
try {
97+
DashboardWorkspace workspace = workspaceService.updateWorkspace(
98+
id,
99+
asString(body.get("name")),
100+
// Distinguish "omitted" from "cleared": a present-but-blank value clears
101+
// the field, matching updateDashboard's convention.
102+
body.containsKey("description") ? String.valueOf(Objects.toString(body.get("description"), "")) : null,
103+
body.containsKey("color") ? String.valueOf(Objects.toString(body.get("color"), "")) : null
104+
);
105+
return ResponseEntity.ok(describe(workspace));
106+
} catch (ResponseStatusException e) {
107+
throw e;
108+
} catch (Exception e) {
109+
log.error("Error updating dashboard workspace", e);
110+
return failure("Failed to update workspace");
111+
}
112+
}
113+
114+
@DeleteMapping("/{id}")
115+
public ResponseEntity<?> deleteWorkspace(@PathVariable UUID id) {
116+
try {
117+
workspaceService.deleteWorkspace(id);
118+
return ResponseEntity.ok(Map.of("success", true));
119+
} catch (ResponseStatusException e) {
120+
throw e;
121+
} catch (Exception e) {
122+
log.error("Error deleting dashboard workspace", e);
123+
return failure("Failed to delete workspace");
124+
}
125+
}
126+
127+
@GetMapping("/{id}/dashboards")
128+
public ResponseEntity<?> listDashboards(@PathVariable UUID id) {
129+
try {
130+
return ResponseEntity.ok(workspaceService.listDashboards(id));
131+
} catch (ResponseStatusException e) {
132+
throw e;
133+
} catch (Exception e) {
134+
log.error("Error listing workspace dashboards", e);
135+
return failure("Failed to load dashboards");
136+
}
137+
}
138+
139+
@GetMapping("/{id}/members")
140+
public ResponseEntity<?> listMembers(@PathVariable UUID id) {
141+
try {
142+
List<DashboardWorkspaceMember> members = workspaceService.listMembers(id);
143+
return ResponseEntity.ok(members.stream().map(this::describeMember).toList());
144+
} catch (ResponseStatusException e) {
145+
throw e;
146+
} catch (Exception e) {
147+
log.error("Error listing workspace members", e);
148+
return failure("Failed to load members");
149+
}
150+
}
151+
152+
@PostMapping("/{id}/members")
153+
public ResponseEntity<?> addMember(@PathVariable UUID id, @RequestBody Map<String, Object> body) {
154+
try {
155+
DashboardWorkspaceMember member = workspaceService.addMember(
156+
id, asString(body.get("username")), asString(body.get("workspaceRole")));
157+
return ResponseEntity.ok(describeMember(member));
158+
} catch (ResponseStatusException e) {
159+
throw e;
160+
} catch (Exception e) {
161+
log.error("Error adding workspace member", e);
162+
return failure("Failed to add member");
163+
}
164+
}
165+
166+
@DeleteMapping("/{id}/members/{username}")
167+
public ResponseEntity<?> removeMember(@PathVariable UUID id, @PathVariable String username) {
168+
try {
169+
workspaceService.removeMember(id, username);
170+
return ResponseEntity.ok(Map.of("success", true));
171+
} catch (ResponseStatusException e) {
172+
throw e;
173+
} catch (Exception e) {
174+
log.error("Error removing workspace member", e);
175+
return failure("Failed to remove member");
176+
}
177+
}
178+
179+
/** Move a dashboard into a workspace, or out of one with a null/blank workspaceId. */
180+
@PutMapping("/dashboards/{dashboardId}")
181+
public ResponseEntity<?> moveDashboard(@PathVariable UUID dashboardId, @RequestBody Map<String, Object> body) {
182+
try {
183+
String raw = asString(body.get("workspaceId"));
184+
UUID target = raw == null ? null : UUID.fromString(raw);
185+
SavedDashboard dashboard = workspaceService.moveDashboard(dashboardId, target);
186+
return ResponseEntity.ok(Map.of(
187+
"success", true,
188+
"dashboardId", dashboard.getId().toString(),
189+
"workspaceId", dashboard.getWorkspaceId() == null ? "" : dashboard.getWorkspaceId().toString()
190+
));
191+
} catch (IllegalArgumentException e) {
192+
return ResponseEntity.badRequest().body(Map.of("success", false, "message", "Invalid workspace id"));
193+
} catch (ResponseStatusException e) {
194+
throw e;
195+
} catch (Exception e) {
196+
log.error("Error moving dashboard between workspaces", e);
197+
return failure("Failed to move dashboard");
198+
}
199+
}
200+
201+
private Map<String, Object> describe(DashboardWorkspace workspace) {
202+
Map<String, Object> body = new LinkedHashMap<>();
203+
body.put("id", workspace.getId().toString());
204+
body.put("connectionId", workspace.getConnectionId());
205+
body.put("name", workspace.getName());
206+
body.put("description", workspace.getDescription());
207+
body.put("color", workspace.getColor());
208+
body.put("createdBy", workspace.getCreatedBy());
209+
body.put("createdAt", workspace.getCreatedAt());
210+
body.put("updatedAt", workspace.getUpdatedAt());
211+
body.put("dashboardCount", savedDashboardRepository.countByWorkspaceId(workspace.getId()));
212+
return body;
213+
}
214+
215+
private Map<String, Object> describeMember(DashboardWorkspaceMember member) {
216+
Map<String, Object> body = new LinkedHashMap<>();
217+
body.put("id", member.getId().toString());
218+
body.put("username", member.getUsername());
219+
body.put("workspaceRole", member.getWorkspaceRole().name());
220+
body.put("addedBy", member.getAddedBy());
221+
body.put("createdAt", member.getCreatedAt());
222+
return body;
223+
}
224+
225+
private static ResponseEntity<Map<String, Object>> failure(String message) {
226+
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
227+
.body(Map.of("success", false, "message", message));
228+
}
229+
230+
private static String asString(Object value) {
231+
if (value == null) return null;
232+
String s = String.valueOf(value).trim();
233+
return s.isEmpty() ? null : s;
234+
}
235+
}

0 commit comments

Comments
 (0)