Commit 08b2fb4
authored
fix(admin): forward session cookie on KeyViz fan-out so peers do not 401 (#692)
## Summary
**Hotfix for the screenshot a user shared after enabling fan-out** — the
heatmap collapsed to "1 of 6 nodes responded" with every peer returning
`401 missing session cookie`.
**Root cause**: PR #686 shipped fan-out with the design assumption (#685
§3) that peers would accept anonymous calls on a private network. But
the receiving side runs the same `SessionAuth` middleware as the browser
path, so anonymous fan-out calls are rejected with 401. The "anonymous
on a private network" framing was wrong: the implementation didn't put a
bypass on the peer side, so anonymous calls never went through
end-to-end.
**Fix**: forward the inbound user's cookies on every peer call. Cluster
nodes already share `--adminSessionSigningKey` for HA, so a cookie
minted on node A is verifiable on node B — no new infrastructure needed.
## What changed
- `KeyVizFanout.Run` gains a `cookies []*http.Cookie` parameter; nil
preserves the legacy behaviour for tests / future non-browser callers.
- `KeyVizHandler` passes `r.Cookies()` so a SPA poll forwards
`admin_session` (and `admin_csrf`, harmless on a GET) transparently.
- `TestKeyVizFanoutRunForwardsCookies` pins that both cookies reach the
peer verbatim.
- Design doc §3 rewritten: documents the cookie-forwarding scheme + the
operator-trust assumption that `--keyvizFanoutNodes` points at trusted
hosts only. Phase 2-C+ will replace cookie forwarding with a dedicated
inter-node token to decouple browser-session expiry from inter-node call
validity.
## Five-lens self-review
1. **Data loss** — n/a; auth path change.
2. **Concurrency / distributed** — Cookies are passed by reference into
per-peer goroutines but the slice is read-only inside `fetchPeer` (each
goroutine `req.AddCookie`s independently). No shared mutation.
3. **Performance** — Adds N `req.AddCookie` calls per fan-out request.
Trivial.
4. **Data consistency** — Auth-only change; merge semantics unchanged.
5. **Test coverage** — New `TestKeyVizFanoutRunForwardsCookies` asserts
both cookies reach the peer with the original values. Existing fan-out
tests pass `nil` and continue to exercise the unauthenticated path the
existing code handled (peer 401s, request reports `ok=false`).
## Test plan
- [x] `go test -race -count=1 ./internal/admin/...` — clean
- [x] `golangci-lint run ./internal/admin/...` — clean
- [ ] Manual: 6-node cluster with shared
`ELASTICKV_ADMIN_SESSION_SIGNING_KEY`, `--keyvizFanoutNodes` set on
every node. Heatmap should now show "6 of 6 nodes" instead of "1 of 6".
## Trust / threat model note
Cookie forwarding is safe when (a) every peer is operator-configured and
trusted, and (b) the network is private. **Do NOT point
`--keyvizFanoutNodes` at an untrusted host** — the user's admin session
would be replayed there. The design doc §3 update is explicit about
this. Phase 2-C+ removes this footgun by switching to a dedicated
inter-node token.
Closes the production-impacting symptom from screenshot review.5 files changed
Lines changed: 221 additions & 31 deletions
File tree
- docs/design
- internal/admin
Lines changed: 39 additions & 16 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
132 | 132 | | |
133 | 133 | | |
134 | 134 | | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
151 | 174 | | |
152 | 175 | | |
153 | 176 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
37 | 45 | | |
38 | 46 | | |
39 | 47 | | |
| |||
190 | 198 | | |
191 | 199 | | |
192 | 200 | | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
193 | 220 | | |
194 | 221 | | |
195 | 222 | | |
| |||
205 | 232 | | |
206 | 233 | | |
207 | 234 | | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
208 | 243 | | |
209 | 244 | | |
210 | | - | |
| 245 | + | |
211 | 246 | | |
212 | 247 | | |
213 | 248 | | |
| |||
218 | 253 | | |
219 | 254 | | |
220 | 255 | | |
221 | | - | |
| 256 | + | |
222 | 257 | | |
223 | 258 | | |
224 | 259 | | |
| |||
260 | 295 | | |
261 | 296 | | |
262 | 297 | | |
263 | | - | |
| 298 | + | |
264 | 299 | | |
265 | 300 | | |
266 | 301 | | |
| |||
274 | 309 | | |
275 | 310 | | |
276 | 311 | | |
277 | | - | |
| 312 | + | |
278 | 313 | | |
279 | 314 | | |
280 | 315 | | |
281 | 316 | | |
282 | 317 | | |
283 | 318 | | |
284 | 319 | | |
285 | | - | |
| 320 | + | |
286 | 321 | | |
287 | 322 | | |
288 | 323 | | |
| |||
292 | 327 | | |
293 | 328 | | |
294 | 329 | | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
295 | 338 | | |
296 | 339 | | |
297 | 340 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
152 | 153 | | |
153 | 154 | | |
154 | 155 | | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
155 | 221 | | |
156 | 222 | | |
157 | 223 | | |
| |||
176 | 242 | | |
177 | 243 | | |
178 | 244 | | |
179 | | - | |
| 245 | + | |
180 | 246 | | |
181 | 247 | | |
182 | 248 | | |
| |||
209 | 275 | | |
210 | 276 | | |
211 | 277 | | |
212 | | - | |
| 278 | + | |
213 | 279 | | |
214 | 280 | | |
215 | 281 | | |
| |||
238 | 304 | | |
239 | 305 | | |
240 | 306 | | |
241 | | - | |
| 307 | + | |
242 | 308 | | |
243 | 309 | | |
244 | 310 | | |
| |||
258 | 324 | | |
259 | 325 | | |
260 | 326 | | |
261 | | - | |
| 327 | + | |
262 | 328 | | |
263 | 329 | | |
264 | 330 | | |
| |||
322 | 388 | | |
323 | 389 | | |
324 | 390 | | |
325 | | - | |
| 391 | + | |
326 | 392 | | |
327 | 393 | | |
328 | 394 | | |
| |||
369 | 435 | | |
370 | 436 | | |
371 | 437 | | |
372 | | - | |
| 438 | + | |
373 | 439 | | |
374 | 440 | | |
375 | 441 | | |
| |||
416 | 482 | | |
417 | 483 | | |
418 | 484 | | |
419 | | - | |
| 485 | + | |
420 | 486 | | |
421 | 487 | | |
422 | 488 | | |
| |||
458 | 524 | | |
459 | 525 | | |
460 | 526 | | |
461 | | - | |
| 527 | + | |
462 | 528 | | |
463 | 529 | | |
464 | 530 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
177 | 177 | | |
178 | 178 | | |
179 | 179 | | |
180 | | - | |
181 | | - | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
182 | 192 | | |
183 | 193 | | |
184 | 194 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
450 | 450 | | |
451 | 451 | | |
452 | 452 | | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
0 commit comments