Feat/workspaces and roles

CLAUDE.md

@@ -210,6 +210,99 @@ returns a number).
 4. **Tooltips**: Always use `HelpTooltip` component, never plain `title` attributes.
 5. **Design**: Minimal black/white/grey palette, Inter font, subtle transitions. See UX guidelines in full CLAUDE.md.
 
+### Roles, Permissions & Custom Roles
+
+Roles are **not a hierarchy**. The old model ranked `DEVELOPER < ADMIN` and compared
+`ordinal()`; the shipped roles deliberately overlap without nesting, so an ordering
+comparison has no meaning and `Role.isAtLeast` is gone.
+
+| Role | Sections | Notes |
+|---|---|---|
+| `ADMIN` | everything | Fixed point: holds **every** permission; overrides against it are refused, so the last admin cannot be locked out of user management. |
+| `DBA` | all menus + connection settings | **No** user creation / invite codes / role management. |
+| `DATA_ENGINEER` | Agent, Dashboards, Editor | No Digest, no Performance. |
+| `DEVELOPER` | Agent, Digest, Dashboards, Performance, Editor | No connection settings. |
+| custom | whatever an admin ticks | `custom_roles` rows; the `code` is written to `users.role`. |
+
+- **Permissions are the unit of authorization.** `Permission` carries the built-in roles
+  that hold it by default (`defaultRoles`); one `VIEW_*` permission per sidebar section
+  (`VIEW_AGENT`, `VIEW_DASHBOARDS`, `VIEW_DIGEST`, `VIEW_BRAIN`, `VIEW_PERFORMANCE`,
+  `VIEW_EDITOR`). The frontend gates nav on those codes (`SECTION_PERMISSION` in
+  `src/lib/features.js`), not on a minimum role.
+- **A "role code" is either a built-in `Role` name or a `CustomRole.code`** — they share
+  the `users.role` namespace, so `CustomRoleService` refuses a code colliding with a
+  built-in one. `Role.fromString` returns **null** for anything unrecognised instead of
+  collapsing to DEVELOPER: mapping a custom role onto a built-in one would hand its
+  holders the wrong permissions. Use `PermissionService.getEffectivePermissions(roleCode)`
+  — `User.getRoleEnum()` is null for a custom role and `Role.getPermissions()` skips
+  overrides.
+- **Every token-minting path must resolve by role code.** `AuthSessionService`,
+  `PasswordlessAuthService`, `AuthInternalController`, `CustomUserDetailsService` and the
+  `/auth/me` payload all use `user.getRoleCode()` + `PermissionService`; `JwtUtil` gained
+  a `String roleCode` overload for exactly this. A `Role`-typed path cannot represent a
+  custom role, so a custom-role user would silently get the wrong claim.
+- **An unknown role code grants nothing** rather than falling back — a deleted custom role
+  must not become silent Developer access. Deleting a custom role is refused while any
+  user still holds it.
+- `RolePermissionOverride.role` is now a role-code **string** (same column), so overrides
+  work for custom roles too. Built-in role permission sets are code, not data: the API
+  refuses to edit them directly and points at overrides instead, so an admin's change
+  survives an upgrade.
+
+### Connection access levels & the create-connection guard
+
+- **There is one access level.** `ConnectionAccessLevel.CHAT_EDITOR` is `@Deprecated` and
+  retained only so pre-existing rows parse; `fromString` folds it (and a blank value) into
+  `FULL_CONTENT`, and `ConnectionAccessService.resolveAccess` returns `FULL_CONTENT` for
+  **every** grant. Assigning a connection therefore implies content access — no migration
+  was needed, legacy rows upgrade themselves on read. The "Full Access" / "Chat + Editor"
+  badges are gone; only Owner/Admin are surfaced.
+- **`AccessControlServiceTest` cannot prove anything about this.** It stubs
+  `resolveAccess` to return a fixed `EffectiveConnectionAccess`, so its CHAT_EDITOR case
+  passes vacuously no matter what the resolver does. `ConnectionAccessLevelCollapseTest`
+  exercises the real path — add coverage there, not to the stubbed test.
+- **`POST /connections` had no authorization at all.** It went straight to test-and-save,
+  so any authenticated user could create — then edit and delete — their own connection
+  (verified live: the row persisted with `owner_username = analyst` for a DATA_ENGINEER).
+  Hiding the sidebar button is not a control. It now calls
+  `accessControlService.assertCanManageConnections()`, which is **permission-based, not
+  admin-only**, so DBA and any custom role holding `MANAGE_CONNECTIONS` still work.
+  Creation is not scoped to a connection id, so none of the `assertCanManage*Connection*`
+  helpers apply — a new unscoped endpoint needs this guard explicitly.
+- **Settings and Connections are admin surfaces in the UI.** `SettingsModal` and
+  `ManageConnectionsModal` each refuse to render without the relevant permission, enforced
+  *inside* the component rather than only at the call site: both are opened from several
+  places, and gating each entry point separately means the next one silently reopens the
+  hole. Hiding Settings also removes MCP tokens from those roles — that is intended.
+
+### Dashboard workspaces
+
+`DashboardWorkspace` groups dashboards within one connection and carries its own member
+list (`DashboardWorkspaceMember`, keyed by **username** to match `connection_access_grant`
+so "View as" resolves membership as the target user).
+
+- **The rule is an AND, and it only ever narrows.** Connection access is checked first and
+  unchanged (`assertCanReadConnectionContent`); workspace membership is an *additional*
+  gate. Adding someone to a workspace can 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, matching how 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, never deletes them** (the FK is
+  deliberately non-cascading). Removing the last MANAGER is refused, otherwise the
+  workspace could never be changed again by anyone but an admin.
+- `DashboardWorkspaceService.filterReadable` resolves a whole list in one membership
+  query; use it for any new dashboard-list endpoint rather than checking per row.
+- **`/saved-dashboards` had no connection authorization at all** before this change —
+  create, list, get, update and delete took a caller-supplied `connectionId`/id and
+  checked nothing, so any authenticated user could read every dashboard on every
+  connection (verified live against a running install, not inferred). All of them now
+  assert connection access *and* the workspace gate; `DashboardAlertController` does the
+  same through its single `requireDashboard` choke point. This is the same
+  "authentication is not authorization" trap `BrainController` documents — there is still
+  no filter doing it for you.
+
 ### Admin profile switch
 Admins can **View as** a sub-user from the top-right of the home layout (`ProfileSwitch`) to verify connection ACLs, chat/editor policies, and role-gated nav.
 

backend/src/main/java/com/dbaagent/config/RolePermissionConstraintInitializer.java

@@ -0,0 +1,95 @@
+package com.dbaagent.config;
+
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.DependsOn;
+import org.springframework.jdbc.core.JdbcTemplate;
+
+import javax.sql.DataSource;
+
+/**
+ * Drops the stale CHECK constraints Hibernate generated for
+ * {@code role_permission_overrides} under the original two-role, twenty-permission
+ * enums.
+ *
+ * <p>{@code ddl-auto=update} adds columns and tables but <em>never drops a constraint it
+ * previously created</em>, so on any database created before the role model changed both
+ * checks survive and reject every new value. Inserting an override for the DBA role, or
+ * for any of the new section permissions, fails with:
+ *
+ * <pre>
+ *   ERROR: new row for relation "role_permission_overrides" violates check constraint
+ *          "role_permission_overrides_permission_code_check"
+ * </pre>
+ *
+ * <p>That was observed against a live install, not inferred — the tables looked correct
+ * and only an actual INSERT revealed it. There is no Flyway runtime in this repo
+ * (see CLAUDE.md), so this initializer is what actually applies the matching statements
+ * in {@code V117__create_dashboard_workspaces_and_custom_roles.sql}.
+ *
+ * <p>Dropping rather than rewriting the constraints is deliberate: the {@code Role} /
+ * {@code Permission} enums plus {@code PermissionService} are the authority for these
+ * values, and a database-level copy of an enum has to be re-migrated on every future
+ * addition — which is precisely how this broke.
+ */
+@Configuration
+@Slf4j
+public class RolePermissionConstraintInitializer {
+
+    private static final String TABLE = "role_permission_overrides";
+
+    @Bean("rolePermissionConstraintBootstrap")
+    @DependsOn("entityManagerFactory")
+    public Object rolePermissionConstraintBootstrap(DataSource dataSource) {
+        JdbcTemplate jdbc = new JdbcTemplate(dataSource);
+
+        if (!tableExists(jdbc, TABLE)) {
+            return new Object();
+        }
+
+        int dropped = 0;
+        dropped += dropCheckIfPresent(jdbc, TABLE + "_role_check");
+        dropped += dropCheckIfPresent(jdbc, TABLE + "_permission_code_check");
+
+        // Widen the role column so a custom role code fits; harmless if already wide.
+        try {
+            jdbc.execute("ALTER TABLE " + TABLE + " ALTER COLUMN role TYPE VARCHAR(64)");
+        } catch (RuntimeException e) {
+            log.warn("Could not widen {}.role: {}", TABLE, e.getMessage());
+        }
+
+        if (dropped > 0) {
+            log.info("Dropped {} stale CHECK constraint(s) on {} left over from the previous role model",
+                dropped, TABLE);
+        }
+        return new Object();
+    }
+
+    private int dropCheckIfPresent(JdbcTemplate jdbc, String constraintName) {
+        Integer count = jdbc.queryForObject("""
+                SELECT COUNT(*)
+                FROM information_schema.table_constraints
+                WHERE table_schema = 'public' AND table_name = ? AND constraint_name = ?
+                """, Integer.class, TABLE, constraintName);
+        if (count == null || count == 0) {
+            return 0;
+        }
+        try {
+            jdbc.execute("ALTER TABLE " + TABLE + " DROP CONSTRAINT IF EXISTS " + constraintName);
+            return 1;
+        } catch (RuntimeException e) {
+            log.warn("Could not drop stale constraint {}: {}", constraintName, e.getMessage());
+            return 0;
+        }
+    }
+
+    private boolean tableExists(JdbcTemplate jdbc, String tableName) {
+        Integer count = jdbc.queryForObject("""
+                SELECT COUNT(*)
+                FROM information_schema.tables
+                WHERE table_schema = 'public' AND table_name = ?
+                """, Integer.class, tableName);
+        return count != null && count > 0;
+    }
+}

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

@@ -167,7 +167,8 @@ public ResponseEntity<?> completeGoogleLogin(
                 return redirectToFrontend("/login?error=" + urlEncode(result.message()));
             }
 
-            if (result.sessionAuthentication() != null && result.user() != null && result.role() != null) {
+            // Gate on roleCode, not role: role is null for a custom-role user.
+            if (result.sessionAuthentication() != null && result.user() != null && result.roleCode() != null) {
                 authSessionService.writeSessionCookies(httpResponse, result.sessionAuthentication());
                 return redirectToFrontend("/dashboard");
             }
@@ -224,8 +225,8 @@ public ResponseEntity<?> refreshSession(HttpServletRequest httpRequest, HttpServ
         }
         Map<String, Object> payload = toAuthPayload(
             effectiveUser,
-            effectiveUser.getRoleEnum(),
-            permissionService.getEffectivePermissionCodes(effectiveUser.getRoleEnum())
+            effectiveUser.getRoleCode(),
+            permissionService.getEffectivePermissionCodes(effectiveUser.getRoleCode())
         );
         impersonationService.decorateAuthPayload(httpRequest, user, payload);
         return ResponseEntity.ok(payload);
@@ -332,9 +333,9 @@ public ResponseEntity<?> getCurrentUser(HttpServletRequest httpRequest) {
             return ResponseEntity.status(401).body(Map.of("message", "Not authenticated"));
         }
         User user = currentUserEntity();
-        Role role = user.getRoleEnum();
-        Set<String> permissions = permissionService.getEffectivePermissionCodes(role);
-        Map<String, Object> response = toAuthPayload(user, role, permissions);
+        String roleCode = user.getRoleCode();
+        Set<String> permissions = permissionService.getEffectivePermissionCodes(roleCode);
+        Map<String, Object> response = toAuthPayload(user, roleCode, permissions);
         impersonationService.decorateAuthPayload(httpRequest, user, response);
         return ResponseEntity.ok(response);
     }
@@ -378,13 +379,17 @@ private ResponseEntity<?> authResponse(PasswordlessAuthService.AuthFlowResult re
         if (!result.success()) {
             return ResponseEntity.status(400).body(Map.of("message", result.message()));
         }
-        if (result.sessionAuthentication() != null && result.user() != null && result.role() != null) {
+        // Gate on roleCode, not role: result.role() is null for a user holding a custom
+        // role, which sent an otherwise-successful login down the "challenge required"
+        // branch below and then NPE'd in Map.of on a null challengeId — a 500 on every
+        // custom-role login. Observed live, not inferred.
+        if (result.sessionAuthentication() != null && result.user() != null && result.roleCode() != null) {
             authSessionService.writeSessionCookies(httpResponse, result.sessionAuthentication());
             authSessionService.clearImpersonationCookie(httpResponse);
             Set<String> permissionNames = result.permissions() == null ? Set.of() : result.permissions().stream()
                 .map(Enum::name)
                 .collect(Collectors.toSet());
-            return ResponseEntity.ok(toAuthPayload(result.user(), result.role(), permissionNames));
+            return ResponseEntity.ok(toAuthPayload(result.user(), result.roleCode(), permissionNames));
         }
         return ResponseEntity.ok(Map.of(
             "challengeId", result.nextChallengeId(),
@@ -401,10 +406,19 @@ private ResponseEntity<?> authError(ResponseStatusException e) {
     }
 
     private Map<String, Object> toAuthPayload(User user, Role role, Set<String> permissions) {
+        return toAuthPayload(user, role != null ? role.name() : user.getRoleCode(), permissions);
+    }
+
+    /**
+     * Auth payload keyed by role <em>code</em>, so a user holding a custom role reports
+     * that role rather than the nearest built-in one.
+     */
+    private Map<String, Object> toAuthPayload(User user, String roleCode, Set<String> permissions) {
         Map<String, Object> response = new LinkedHashMap<>();
         response.put("username", user.getUsername());
         response.put("email", user.getEmail());
-        response.put("role", role.name());
+        response.put("role", roleCode);
+        response.put("roleName", permissionService.describeRole(roleCode));
         response.put("permissions", permissions);
         response.put("emailVerified", user.isEmailVerified());
         response.put("accountStatus", user.getAccountStatus());

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

@@ -60,11 +60,11 @@ public ResponseEntity<?> issueToken(
                 .body(Map.of("message", "Admin user not found"));
         }
 
-        Role role = admin.getRoleEnum();
-        Set<Permission> permissions = permissionService.getEffectivePermissions(role);
+        String roleCode = admin.getRoleCode();
+        Set<Permission> permissions = permissionService.getEffectivePermissions(roleCode);
 
         AuthSessionService.SessionAuthentication session = authSessionService.createSession(
-            admin, role, permissions,
+            admin, roleCode, permissions,
             request.getRemoteAddr(),
             "DeepSQL-TestSuite/1.0",
             true

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

@@ -189,6 +189,12 @@ private ConnectionRequest mergeTestRequest(ConnectionRequest saved, ConnectionRe
     public ResponseEntity<Map<String, Object>> saveConnection(@RequestBody ConnectionRequest request) {
         Map<String, Object> response = new HashMap<>();
         try {
+            // Creating a connection is not scoped to an existing connection id, so none of
+            // the assertCanManage*Connection* checks apply here — this endpoint had no
+            // authorization at all, and any authenticated user could add (then edit and
+            // delete) their own connection. Hiding the Connections button did not stop it.
+            accessControlService.assertCanManageConnections();
+
             // Test connection with privilege checks
             ConnectionTestResult result = connectionService.testConnectionWithPrivileges(request);
 

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

@@ -4,6 +4,7 @@
 import com.dbaagent.model.SavedDashboard;
 import com.dbaagent.service.DashboardAlertService;
 import com.dbaagent.service.SavedDashboardService;
+import com.dbaagent.service.DashboardWorkspaceService;
 import com.dbaagent.service.security.AccessControlService;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -25,6 +26,7 @@ public class DashboardAlertController {
     private final DashboardAlertService alertService;
     private final SavedDashboardService savedDashboardService;
     private final AccessControlService accessControlService;
+    private final DashboardWorkspaceService dashboardWorkspaceService;
 
     @PostMapping
     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
         }
     }
 
+    /**
+     * The single point every handler here resolves a dashboard through, so the workspace
+     * membership gate applies to all of them at once. The connection check stays with
+     * each caller because read and write paths need different assertions.
+     */
     private SavedDashboard requireDashboard(UUID dashboardId) {
-        return savedDashboardService.getDashboardById(dashboardId)
+        SavedDashboard dashboard = savedDashboardService.getDashboardById(dashboardId)
             .orElseThrow(() -> new IllegalArgumentException("Dashboard not found"));
+        dashboardWorkspaceService.assertCanReadDashboard(dashboard);
+        return dashboard;
     }
 }