Skip to content

Commit 9577e9d

Browse files
feat: pin a connection as your per-user default
Adds a pin toggle in Manage Connections and in the sidebar connection switcher. A pinned connection is the one DeepSQL opens on every load. The pin is per user, not a flag on the connection. A connection can be shared through connection_access_grant, so a column on database_connection would let one person's choice decide what everyone else opens on — and shared connections are canManageConfig=false for their recipients, so exactly the people who most want a default could not set one. One pin per user, enforced by a unique constraint on connection_pin.username; pinning a second connection moves the row rather than adding one. PUT|DELETE /connections/{id}/pin are gated on assertCanUseConnection, not assertCanManageConnectionConfig: choosing where you land is a preference, not a change to the connection. GET /connections carries the pinned flag per caller, so no surface needs a second request, and deleteConnection clears every pin on the connection alongside its grants. The pin has to beat an already-selected connection, not just an empty one. useDashboardStore persists connectionId, so after a reload something is always selected and the existing auto-select never ran. useConnectionManager now applies the pin once per page load, tracked at module scope rather than in a ref — the hook is called from a dozen sections, and a per-instance guard would let a later-mounted section yank the user back to the pin after they deliberately switched. The sidebar toggle is there because Manage Connections needs MANAGE_CONNECTIONS to open at all; without it a Developer or Data Engineer, who typically holds exactly one granted connection, would have no way to set a default. ConnectionScopedAuthorizationSafetyTest now flags GET /connections, because the handler resolves the caller's pinned connection id and the scanner matches (?i)connection_?id anywhere in a handler body. That endpoint takes no arguments at all, so it is exempted in AUTHORIZED_ELSEWHERE and connectionListingTakesNoCallerSuppliedId re-derives the claim, so the exemption cannot rot into cover for a real gap. The scanner was not weakened and no meaningless assert was added. Verified against a live backend on a throwaway stack: ddl-auto creates connection_pin with its unique index; pinning a second connection flips the first; unpinning a non-pinned connection leaves the real pin alone; deleting a connection clears its pin; a DEVELOPER with no grant gets 403 (not 500); a DEVELOPER holding only a grant pins successfully and their pin does not appear on the admin's list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 819c286 commit 9577e9d

19 files changed

Lines changed: 864 additions & 38 deletions

CLAUDE.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,51 @@ so "View as" resolves membership as the target user).
498498
"authentication is not authorization" trap `BrainController` documents — there is still
499499
no filter doing it for you.
500500

501+
### Default connection (pinning)
502+
503+
A user can pin one connection as their default, from the pin column in **Manage
504+
Connections** or the pin toggle in the sidebar connection switcher.
505+
506+
- **The pin is per user, not per connection.** `connection_pin` keys on username with a
507+
unique constraint (`V120__create_connection_pin.sql`; applied by `ddl-auto` from the
508+
`ConnectionPin` entity — verified on a scratch database, the table and its unique index
509+
are created on boot). A column on `database_connection` would have been wrong twice
510+
over: a connection shared through `connection_access_grant` would let one user's choice
511+
decide what everyone else opens on, and a shared connection is `canManageConfig=false`
512+
for its recipients, so exactly the people who most want a default could not set one.
513+
- **One pin per user is the point.** `ConnectionPinService.pin` moves the existing row
514+
rather than inserting a second; the unique constraint is the backstop, and a losing
515+
concurrent insert re-reads and updates instead of surfacing a 500.
516+
- **`PUT|DELETE /connections/{id}/pin` are gated on `assertCanUseConnection`**, not
517+
`assertCanManageConnectionConfig` — choosing where you land is a preference, not a
518+
change to the connection. Verified live: a DEVELOPER holding only a grant on a
519+
connection (`canManageConfig: false`) pins it and gets 200, while the same user pinning
520+
a connection they hold no grant on gets **403, not 500** — the `ResponseStatusException`
521+
rethrow before the catch-all is doing its job.
522+
- **Unpin is scoped to the connection named.** A stale click in a background tab must not
523+
clear a pin the user has since moved elsewhere.
524+
- **`GET /connections` carries `pinned` per caller**, so no surface needs a second
525+
request, and two users listing the same shared connection see different values —
526+
confirmed live. `deleteConnection` clears every pin on the connection alongside its
527+
grants.
528+
- **`ConnectionScopedAuthorizationSafetyTest` flags `GET /connections` now**, because the
529+
handler resolves the caller's *pinned* connection id and the scanner matches
530+
`(?i)connection_?id` anywhere in a handler body. That endpoint takes no arguments at all
531+
— it returns whatever `getConnectionsForUser(username, isAdmin)` gives — so it is in
532+
`AUTHORIZED_ELSEWHERE`, and `connectionListingTakesNoCallerSuppliedId` re-derives that
533+
claim so the exemption cannot rot into cover for a real gap. Do not resolve this by
534+
adding a meaningless assert, and do not weaken the scanner.
535+
- **The pin must beat an already-selected connection, not just an empty one.**
536+
`useDashboardStore` persists `connectionId`, so after a reload something is always
537+
selected — the original auto-select ran only when nothing was. `useConnectionManager`
538+
therefore applies the pin once per page load (`pinAppliedThisLoad`, module scope, reset
539+
by `resetConnectionPinApplied()` in the auth reset). Module scope and not a ref: the
540+
hook is called from a dozen sections, and a per-instance guard would let a
541+
later-mounted section yank the user back to the pin after they deliberately switched.
542+
Switching mid-session still sticks; the pin re-applies on the next load.
543+
- Pinned connections sort first in `useConnectionManager`, so every consumer — the sidebar
544+
switcher included — shows the default at the top.
545+
501546
### Admin profile switch
502547
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.
503548

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

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.dbaagent.repository.ConnectionInitHistoryRepository;
1010
import com.dbaagent.repository.ConnectionInitStatusRepository;
1111
import com.dbaagent.repository.SchemaDocumentationRepository;
12+
import com.dbaagent.service.ConnectionPinService;
1213
import com.dbaagent.service.ConnectionService;
1314
import com.dbaagent.service.scheduler.BrainInitSchedulerService;
1415
import com.dbaagent.service.scheduler.BrainJobsService;
@@ -45,6 +46,7 @@ public class ConnectionController {
4546
private final AccessControlService accessControlService;
4647
private final ConnectionAccessService connectionAccessService;
4748
private final com.dbaagent.repository.ConnectionAccessGrantRepository connectionAccessGrantRepository;
49+
private final ConnectionPinService connectionPinService;
4850

4951
@PostMapping("/test")
5052
public ResponseEntity<Map<String, Object>> testConnection(@RequestBody ConnectionRequest request) {
@@ -429,8 +431,10 @@ public ResponseEntity<List<ConnectionSummaryResponse>> getAllConnections() {
429431
String username = accessControlService.getCurrentUsername();
430432
boolean isAdmin = accessControlService.isCurrentUserAdmin();
431433
List<DatabaseConnection> connections = credentialService.getConnectionsForUser(username, isAdmin);
434+
// One lookup for the whole list rather than one per row.
435+
String pinnedId = connectionPinService.pinnedConnectionId(username).orElse(null);
432436
List<ConnectionSummaryResponse> decryptedConnections = connections.stream()
433-
.map(conn -> toSummary(conn, username, isAdmin))
437+
.map(conn -> toSummary(conn, username, isAdmin, pinnedId))
434438
.toList();
435439
return ResponseEntity.ok(decryptedConnections);
436440
} catch (org.springframework.web.server.ResponseStatusException e) {
@@ -440,13 +444,62 @@ public ResponseEntity<List<ConnectionSummaryResponse>> getAllConnections() {
440444
}
441445
}
442446

447+
/**
448+
* Pin this connection as the caller's default, replacing any connection they had
449+
* pinned before.
450+
*
451+
* <p>Gated on {@code assertCanUseConnection} rather than
452+
* {@code assertCanManageConnectionConfig}: choosing which database you land on is a
453+
* personal preference, not a change to the connection, and a shared connection is
454+
* config-read-only for its recipients. Requiring manage rights would mean the people
455+
* who most want a default — the ones who were granted exactly one connection —
456+
* could not set one.
457+
*/
458+
@PutMapping("/{id}/pin")
459+
public ResponseEntity<Map<String, Object>> pinConnection(@PathVariable String id) {
460+
Map<String, Object> response = new HashMap<>();
461+
try {
462+
accessControlService.assertCanUseConnection(id);
463+
connectionPinService.pin(accessControlService.requireCurrentUsername(), id);
464+
response.put("success", true);
465+
response.put("pinned", true);
466+
return ResponseEntity.ok(response);
467+
} catch (org.springframework.web.server.ResponseStatusException e) {
468+
throw e;
469+
} catch (Exception e) {
470+
response.put("success", false);
471+
response.put("message", "Failed to pin connection: " + e.getMessage());
472+
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response);
473+
}
474+
}
475+
476+
/** Clear the caller's default, if this connection is the one currently pinned. */
477+
@DeleteMapping("/{id}/pin")
478+
public ResponseEntity<Map<String, Object>> unpinConnection(@PathVariable String id) {
479+
Map<String, Object> response = new HashMap<>();
480+
try {
481+
accessControlService.assertCanUseConnection(id);
482+
connectionPinService.unpin(accessControlService.requireCurrentUsername(), id);
483+
response.put("success", true);
484+
response.put("pinned", false);
485+
return ResponseEntity.ok(response);
486+
} catch (org.springframework.web.server.ResponseStatusException e) {
487+
throw e;
488+
} catch (Exception e) {
489+
response.put("success", false);
490+
response.put("message", "Failed to unpin connection: " + e.getMessage());
491+
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response);
492+
}
493+
}
494+
443495
@DeleteMapping("/{id}")
444496
public ResponseEntity<Map<String, Object>> deleteConnection(@PathVariable String id) {
445497
Map<String, Object> response = new HashMap<>();
446498
try {
447499
accessControlService.assertCanManageConnectionConfig(id);
448500
connectionService.closeConnectionPool(id);
449501
connectionAccessService.deleteAllGrantsForConnection(id);
502+
connectionPinService.clearPinsForConnection(id);
450503
credentialService.deleteConnection(id);
451504
response.put("success", true);
452505
response.put("message", "Connection deleted successfully");
@@ -781,7 +834,7 @@ public ResponseEntity<?> runBrainJob(@PathVariable String id, @PathVariable Stri
781834
}
782835
}
783836

784-
private ConnectionSummaryResponse toSummary(DatabaseConnection conn, String username, boolean isAdmin) {
837+
private ConnectionSummaryResponse toSummary(DatabaseConnection conn, String username, boolean isAdmin, String pinnedConnectionId) {
785838
ConnectionSummaryResponse summary = new ConnectionSummaryResponse();
786839
try {
787840
ConnectionRequest decrypted = credentialService.getDecryptedConnection(conn.getId());
@@ -821,6 +874,7 @@ private ConnectionSummaryResponse toSummary(DatabaseConnection conn, String user
821874
summary.setAccessLevel(resolved.getEffectiveAccess().name());
822875
summary.setCanManageConfig(resolved.canManageConfig());
823876
summary.setCanManageContent(resolved.canManageContent());
877+
summary.setPinned(conn.getId() != null && conn.getId().equals(pinnedConnectionId));
824878
return summary;
825879
}
826880

backend/src/main/java/com/dbaagent/dto/ConnectionSummaryResponse.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,13 @@ public class ConnectionSummaryResponse {
3131
private String accessLevel;
3232
private Boolean canManageConfig;
3333
private Boolean canManageContent;
34+
35+
/**
36+
* Whether the calling user has pinned this connection as their default.
37+
*
38+
* <p>Per caller, not per connection — two users listing the same shared connection
39+
* see different values here. It rides the list response so the UI needs no second
40+
* request to know which row carries the pin.
41+
*/
42+
private Boolean pinned;
3443
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package com.dbaagent.model;
2+
3+
import jakarta.persistence.*;
4+
import lombok.AllArgsConstructor;
5+
import lombok.Data;
6+
import lombok.NoArgsConstructor;
7+
8+
import java.time.LocalDateTime;
9+
10+
/**
11+
* One user's default database connection.
12+
*
13+
* <p>The pin is deliberately <em>per user</em> rather than a flag on
14+
* {@link DatabaseConnection}. A connection can be shared with several people through
15+
* {@code connection_access_grant}, and a default is a personal preference — one user's
16+
* choice must not decide what everyone else opens on. A column on the connection row
17+
* would also put the setting out of reach of exactly the people who need it: a shared
18+
* connection has {@code canManageConfig == false} for its recipients, so they could
19+
* never pin the connection they use every day.
20+
*
21+
* <p>At most one row per user — the unique constraint on {@code username} is what makes
22+
* "always the default" true rather than merely intended. Pinning a second connection
23+
* moves the pin instead of creating a second one; see
24+
* {@code ConnectionPinService.pin}.
25+
*
26+
* <p>{@code connection_id} carries no foreign key, matching
27+
* {@link ConnectionAccessGrant}. {@code ConnectionController.deleteConnection} clears
28+
* pins alongside grants; a pin that outlives its connection is inert anyway, since the
29+
* flag is only ever computed for connections the caller can already see.
30+
*/
31+
@Entity
32+
@Table(
33+
name = "connection_pin",
34+
uniqueConstraints = @UniqueConstraint(
35+
name = "ux_connection_pin_username",
36+
columnNames = {"username"}
37+
)
38+
)
39+
@Data
40+
@NoArgsConstructor
41+
@AllArgsConstructor
42+
public class ConnectionPin {
43+
@Id
44+
@GeneratedValue(strategy = GenerationType.IDENTITY)
45+
private Long id;
46+
47+
@Column(nullable = false)
48+
private String username;
49+
50+
@Column(name = "connection_id", nullable = false, length = 36)
51+
private String connectionId;
52+
53+
@Column(name = "created_at", nullable = false)
54+
private LocalDateTime createdAt;
55+
56+
@Column(name = "updated_at", nullable = false)
57+
private LocalDateTime updatedAt;
58+
59+
@PrePersist
60+
void onCreate() {
61+
LocalDateTime now = LocalDateTime.now();
62+
createdAt = now;
63+
updatedAt = now;
64+
}
65+
66+
@PreUpdate
67+
void onUpdate() {
68+
updatedAt = LocalDateTime.now();
69+
}
70+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.dbaagent.repository;
2+
3+
import com.dbaagent.model.ConnectionPin;
4+
import org.springframework.data.jpa.repository.JpaRepository;
5+
import org.springframework.data.jpa.repository.Query;
6+
import org.springframework.transaction.annotation.Transactional;
7+
8+
import java.util.Optional;
9+
10+
public interface ConnectionPinRepository extends JpaRepository<ConnectionPin, Long> {
11+
12+
/**
13+
* Case-insensitive, matching how {@code ConnectionAccessGrantRepository} resolves
14+
* usernames — a login that differs only in casing must not end up with a second pin
15+
* the unique constraint cannot see.
16+
*/
17+
@Query("select p from ConnectionPin p where lower(p.username) = lower(?1)")
18+
Optional<ConnectionPin> findByUsernameIgnoreCase(String username);
19+
20+
/**
21+
* Derived deletes need their own transaction. Annotating a self-invoked caller does
22+
* nothing — Spring proxies are bypassed by {@code this::} — which is the same trap
23+
* {@code McpTokenRepository.deleteByUserId} documents.
24+
*/
25+
@Transactional
26+
void deleteByConnectionId(String connectionId);
27+
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package com.dbaagent.service;
2+
3+
import com.dbaagent.model.ConnectionPin;
4+
import com.dbaagent.repository.ConnectionPinRepository;
5+
import lombok.RequiredArgsConstructor;
6+
import lombok.extern.slf4j.Slf4j;
7+
import org.springframework.dao.DataIntegrityViolationException;
8+
import org.springframework.stereotype.Service;
9+
import org.springframework.transaction.annotation.Transactional;
10+
11+
import java.util.Optional;
12+
13+
/**
14+
* The per-user default connection.
15+
*
16+
* <p>Callers are responsible for authorizing the connection first — this service takes an
17+
* already-checked id. {@code ConnectionController} calls
18+
* {@code assertCanReadConnectionContent} before every pin write, so a pin cannot be used
19+
* to assert an interest in a connection the caller cannot see.
20+
*/
21+
@Service
22+
@RequiredArgsConstructor
23+
@Slf4j
24+
public class ConnectionPinService {
25+
26+
private final ConnectionPinRepository pinRepository;
27+
28+
/** The connection this user opens by default, if they have chosen one. */
29+
public Optional<String> pinnedConnectionId(String username) {
30+
if (username == null || username.isBlank()) {
31+
return Optional.empty();
32+
}
33+
return pinRepository.findByUsernameIgnoreCase(username).map(ConnectionPin::getConnectionId);
34+
}
35+
36+
/**
37+
* Make {@code connectionId} this user's default, replacing any previous pin.
38+
*
39+
* <p>Moving the existing row rather than inserting a second one is what keeps "the
40+
* default" singular; the unique constraint on {@code username} is the backstop. Two
41+
* pins racing in from different tabs can still collide on that constraint, so the
42+
* loser re-reads and updates instead of surfacing a 500 for what is really a
43+
* last-write-wins preference.
44+
*/
45+
@Transactional
46+
public void pin(String username, String connectionId) {
47+
if (username == null || username.isBlank() || connectionId == null || connectionId.isBlank()) {
48+
return;
49+
}
50+
Optional<ConnectionPin> existing = pinRepository.findByUsernameIgnoreCase(username);
51+
if (existing.isPresent()) {
52+
ConnectionPin pin = existing.get();
53+
if (connectionId.equals(pin.getConnectionId())) {
54+
return;
55+
}
56+
pin.setConnectionId(connectionId);
57+
pinRepository.save(pin);
58+
return;
59+
}
60+
61+
ConnectionPin pin = new ConnectionPin();
62+
pin.setUsername(username);
63+
pin.setConnectionId(connectionId);
64+
try {
65+
pinRepository.save(pin);
66+
} catch (DataIntegrityViolationException e) {
67+
pinRepository.findByUsernameIgnoreCase(username).ifPresent(concurrent -> {
68+
concurrent.setConnectionId(connectionId);
69+
pinRepository.save(concurrent);
70+
});
71+
}
72+
}
73+
74+
/**
75+
* Clear this user's default, but only when it is still the connection they asked to
76+
* unpin. A stale click in another tab must not silently drop a pin the user has since
77+
* moved somewhere else.
78+
*/
79+
@Transactional
80+
public void unpin(String username, String connectionId) {
81+
if (username == null || username.isBlank()) {
82+
return;
83+
}
84+
pinRepository.findByUsernameIgnoreCase(username)
85+
.filter(pin -> connectionId == null || connectionId.equals(pin.getConnectionId()))
86+
.ifPresent(pinRepository::delete);
87+
}
88+
89+
/** Drop every user's pin on a connection that is being deleted. */
90+
public void clearPinsForConnection(String connectionId) {
91+
if (connectionId == null || connectionId.isBlank()) {
92+
return;
93+
}
94+
pinRepository.deleteByConnectionId(connectionId);
95+
}
96+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- Per-user default connection ("pin").
2+
--
3+
-- One row per user: the unique constraint is what makes a pinned connection *the*
4+
-- default rather than one of several. Pinning a second connection moves this row
5+
-- (ConnectionPinService.pin) instead of inserting another.
6+
--
7+
-- Deliberately not a column on database_connection: a connection can be shared with
8+
-- several users via connection_access_grant, and one user's default must not decide
9+
-- what anyone else opens on.
10+
--
11+
-- NOTE: this repository has no Flyway runtime — schema is applied by
12+
-- spring.jpa.hibernate.ddl-auto=update from the ConnectionPin entity. Apply by hand
13+
-- with psql only if you manage schema manually.
14+
CREATE TABLE connection_pin (
15+
id BIGSERIAL PRIMARY KEY,
16+
username VARCHAR(255) NOT NULL,
17+
connection_id VARCHAR(36) NOT NULL,
18+
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
19+
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
20+
);
21+
22+
CREATE UNIQUE INDEX ux_connection_pin_username ON connection_pin (username);
23+
24+
CREATE INDEX idx_connection_pin_connection ON connection_pin (connection_id);

0 commit comments

Comments
 (0)