Commit 27d07ea
fix: clear stale connectionId after impersonation changes (#121)
<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
## Problem
When an admin uses "View as" to impersonate a user who doesn't have
access to the currently selected connection, the UI shows a 403 error
("chat and editor access denied for this connection").
### Reproduction Steps
1. Admin selects `aws-rds-master` connection (only Arun/Gaurav have
grants)
2. Admin uses "View as" to switch to a user without access to that
connection
3. User sees 403 error in Agent/Editor/other connection-scoped sections
### User Experience
The user is stuck on a connection they cannot access, with no clear way
to recover. The connection appears selected in the sidebar, but all API
calls fail with 403.
## Root Cause
The frontend's `useConnectionManager` hook had an early return when
`connectionId` was set, without validating that the connection was
actually in the current user's `connections` array.
When impersonation changes:
1. Backend correctly returns only connections visible to the target user
via `ConnectionAccessService.getVisibleConnections()`
2. Frontend receives the new (filtered) connection list
3. **Bug**: The hook's `useEffect` returned early if `connectionId` was
set, keeping the stale connection
4. Section components then made API calls with the stale `connectionId`,
getting 403
```javascript
// Before (buggy):
useEffect(() => {
if (connections.length === 0) return
if (connectionId) return // Early return without validation!
// ...auto-select logic
}, [connections, connectionId, ...])
```
## Fix
### 1. Core fix in `useConnectionManager.js`
Added validation that `connectionId` exists in the `connections` array.
If not, clear it and auto-select from valid options:
```javascript
// After (fixed):
if (connectionId) {
const stillValid = connections.some((c) => c.id === connectionId)
if (stillValid) return
// Clear the stale connection
localStorage.removeItem('selectedConnectionId')
setConnectionId(null)
}
```
### 2. Defensive guards in section components
Added guards in all connection-scoped sections to prevent rendering with
invalid connections:
- Check `isLoading` first - prevents rendering during connection list
refresh
- Check both `connectionId` AND `selectedConnection` - ensures the
connection is valid
This pattern was applied to all affected sections:
- `AgentChatSection`
- `EditorSection`
- `DashboardsSection`
- `SchemaSection`
- `SchemaDocsSection`
- `SlowQueriesSection`
- `DigestSection`
- `CompanyKnowledgeSection`
- `MonitorSection`
### 3. Backend test added
Added `impersonatingUserWithoutAccessDeniesConnection()` test to verify
the backend correctly denies access when an admin views as a user who
lacks access to a connection.
## Security Considerations
**This fix does NOT weaken security:**
- The backend grant model remains unchanged
- Admin bypass for connection access is preserved
- All API authorization checks (`assertCanReadConnectionContent`,
`assertCanUseChatEditor`, etc.) remain in place
- This fix only ensures the UI doesn't present inaccessible connections
as usable
## Testing
The fix ensures:
1. When impersonation changes, stale connections are cleared
2. Users see a proper "no connection selected" state instead of 403
errors
3. Auto-selection picks from the effective user's accessible connections
### Scenarios Covered
- ✅ Admin can access any connection (admin bypass preserved)
- ✅ Granted user can access assigned connections
- ✅ User without grant sees empty/appropriate state
- ✅ Impersonation where target user lacks access to admin's selected
connection
### Test Results
- `AccessControlServiceTest` passes (including new impersonation test)
- Frontend lint passes (no new errors introduced)
## Note for Stayflexi Deployment
On Stayflexi, only Arun and Gaurav have grants on the `aws-rds-master`
connection. Other users viewing as them or accessing that connection
will now see a proper "no connection selected" state rather than a
confusing 403 error. If broader access is needed, connection grants
should be added via **Manage Connections → Share**.
## Files Changed
| File | Purpose |
|------|---------|
| `src/lib/hooks/useConnectionManager.js` | Core fix: validate
connectionId against connections array |
| `src/components/sections/*.jsx` | Add isLoading and selectedConnection
guards |
| `src/components/sections/*.module.css` | Add loading spinner animation
|
| `backend/.../AccessControlServiceTest.java` | Add test for
impersonation with inaccessible connection |
<!-- CURSOR_AGENT_PR_BODY_END -->
<div><a
href="https://cursor.com/agents/bc-33392497-bd30-5d7a-a359-0dba4888d535?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a
href="https://cursor.com/background-agent?bcId=bc-33392497-bd30-5d7a-a359-0dba4888d535&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>1 parent b4a6165 commit 27d07ea
13 files changed
Lines changed: 218 additions & 31 deletions
File tree
- backend/src/test/java/com/dbaagent/service/security
- src
- components/sections
- lib/hooks
Lines changed: 38 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
359 | 359 | | |
360 | 360 | | |
361 | 361 | | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
362 | 400 | | |
363 | 401 | | |
364 | 402 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
10 | 25 | | |
11 | 26 | | |
12 | 27 | | |
| |||
20 | 35 | | |
21 | 36 | | |
22 | 37 | | |
23 | | - | |
24 | | - | |
| 38 | + | |
| 39 | + | |
25 | 40 | | |
26 | 41 | | |
27 | 42 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
10 | 20 | | |
11 | 21 | | |
12 | 22 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
26 | 36 | | |
27 | 37 | | |
28 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
165 | 165 | | |
166 | 166 | | |
167 | 167 | | |
168 | | - | |
| 168 | + | |
169 | 169 | | |
170 | 170 | | |
171 | 171 | | |
| |||
257 | 257 | | |
258 | 258 | | |
259 | 259 | | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
260 | 280 | | |
261 | 281 | | |
262 | 282 | | |
| |||
309 | 329 | | |
310 | 330 | | |
311 | 331 | | |
312 | | - | |
| 332 | + | |
| 333 | + | |
313 | 334 | | |
314 | 335 | | |
315 | 336 | | |
316 | 337 | | |
317 | 338 | | |
318 | 339 | | |
319 | 340 | | |
320 | | - | |
| 341 | + | |
321 | 342 | | |
322 | 343 | | |
323 | 344 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
10 | 20 | | |
11 | 21 | | |
12 | 22 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
31 | 42 | | |
32 | 43 | | |
33 | 44 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
15 | 25 | | |
16 | 26 | | |
17 | 27 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
10 | 20 | | |
11 | 21 | | |
12 | 22 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
75 | 83 | | |
76 | 84 | | |
77 | 85 | | |
| |||
0 commit comments