Skip to content

Commit 5a4123f

Browse files
brianmclaude
andcommitted
fix: share in-flight auth attempt across ssh sessions and kill abandoned flows
An abandoned OIDC browser flow (browser closed without completing) held the auth mutex for the plugin's full 5-minute timeout, silently blocking every subsequent ssh session behind it. * Coalesce concurrent Auth.Run calls into a shared flight: joiners get the pending user output (auth URL) replayed and share the result, so one browser flow serves all waiting sessions * Kill the auth command (whole process group) when every waiting caller has canceled; thread the gRPC match stream context through so a dead ssh session abandons its auth and CA work * Hold broker lock only around agents-map access, never across auth or CA calls, so concurrent matches can actually share the flight * Always emit the auth URL on fd 4 from the OIDC plugin so joining sessions see where to authenticate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5f65814 commit 5a4123f

9 files changed

Lines changed: 527 additions & 77 deletions

File tree

.tasks/in-progress/yv12wc30.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
yatl_version: 1
3+
title: Fix abandoned OIDC auth flow blocking subsequent ssh sessions
4+
id: yv12wc30
5+
created: 2026-07-13T16:37:06.749142730Z
6+
updated: 2026-07-13T17:37:00.803717631Z
7+
author: Brian McCallister
8+
priority: high
9+
tags:
10+
- bug
11+
- broker
12+
- auth
13+
---
14+
15+
When an OIDC browser auth flow is abandoned (browser closed without completing), Auth.Run in pkg/broker/auth.go holds the mutex for the full auth subprocess lifetime (up to the 5-minute timeout in cmd/epithet/auth_oidc.go). Subsequent ssh sessions block on the mutex at broker.go Match -> auth.Run and never pop the browser or show the auth URL.
16+
17+
Fix:
18+
1. Singleflight: second concurrent caller joins the in-flight auth attempt instead of queueing a second subprocess; re-emit pending fd-4 user output (auth URL) to each joined caller's userOutput.
19+
2. Cancelation: thread context from the match request through Auth.Run -> exec.CommandContext so a dead ssh session kills its auth subprocess and releases the flow — but only when ALL attached waiters have gone away.
20+
21+
---
22+
# Log: 2026-07-13T16:37:06Z Brian McCallister
23+
24+
Created task.
25+
26+
---
27+
# Log: 2026-07-13T16:37:15Z Brian McCallister
28+
29+
Started working.
30+
31+
---
32+
# Log: 2026-07-13T16:37:20Z Brian McCallister
33+
34+
Started working.
35+
36+
---
37+
# Log: 2026-07-13T16:37:40Z Brian McCallister
38+
39+
Started working.
40+
41+
---
42+
# Log: 2026-07-13T16:47:15Z Brian McCallister
43+
44+
COMPLETED: Singleflight + cancelation implemented. Auth.Run(ctx, attrs, userOutput) now coalesces concurrent callers into one authFlight: joiners get fd-4 output replayed (pending auth URL), share the result; subprocess killed (whole process group, SIGKILL) only when all waiters cancel; abandoned flights are not joinable - new callers wait for wind-down then start fresh. Context threaded from gRPC stream.Context() through MatchWithUserOutput -> auth.Run/GetCert. b.lock narrowed to agents-map ops; ensureAgent now takes b.lock itself. Tests: auth_singleflight_test.go (share-one-command, cancel-kills-process, joiner-survives-leader-cancel), all pass with -race. NEXT: manual verification of real browser flow, then commit.
45+
46+
---
47+
# Log: 2026-07-13T17:37:00Z Brian McCallister
48+
49+
COMPLETED: fd-4 fix in pkg/auth/oidc - plugin previously wrote the auth URL only to stderr and only on browser-open failure, so joiners of a shared flight saw nothing. Now always writes 'To authenticate, visit: URL' to fd 4 (user-visible progress) when the callback server is ready; broker replays it to joining sessions. Best-effort write when no fd 4 (standalone runs). DECISION: kept singleflight over concurrent-flights/last-writer-wins because IdP behavior is unknown - refresh-token rotation with reuse detection makes concurrent refreshes unsafe, plus browser popup storms on parallel ssh. NEXT: manual verification of real browser flow, then commit.

pkg/auth/oidc/oidc.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,19 @@ import (
1414
"golang.org/x/oauth2"
1515
)
1616

17+
// userOutput is fd 4 per the epithet auth plugin protocol: user-visible
18+
// progress. The broker streams it to the requesting ssh session and replays
19+
// it to any session that joins an in-flight authentication attempt.
20+
var userOutput io.Writer = os.NewFile(4, "user-output")
21+
22+
// notifyUser writes a user-visible progress message to fd 4. Best effort:
23+
// when running outside the broker (no fd 4), the write fails silently.
24+
func notifyUser(format string, args ...any) {
25+
if userOutput != nil {
26+
_, _ = fmt.Fprintf(userOutput, format, args...)
27+
}
28+
}
29+
1730
// Config holds OIDC authentication configuration.
1831
type Config struct {
1932
IssuerURL string
@@ -170,9 +183,14 @@ func performFullAuth(ctx context.Context, oauth2Config oauth2.Config) (*oauth2.T
170183
// Wait for the local server to be ready, then open browser
171184
select {
172185
case url := <-readyChan:
186+
// Always surface the URL as user-visible progress, so a session
187+
// waiting on this flow (including one that joined it after the
188+
// browser was closed) can complete authentication manually.
189+
notifyUser("To authenticate, visit: %s\n", url)
173190
// Attempt to open the browser
174191
if err := browser.OpenURL(url); err != nil {
175192
// Browser failed to open - user needs the URL to authenticate manually
193+
notifyUser("Could not open browser automatically: %v\n", err)
176194
fmt.Fprintf(os.Stderr, "Could not open browser automatically: %v\n", err)
177195
fmt.Fprintf(os.Stderr, "Please visit: %s\n", url)
178196
}

pkg/auth/oidc/oidc_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package oidc
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/json"
67
"testing"
@@ -9,6 +10,23 @@ import (
910
"golang.org/x/oauth2"
1011
)
1112

13+
// TestNotifyUser tests that user-visible progress goes to the fd 4 writer
14+
// and that a missing fd 4 does not panic.
15+
func TestNotifyUser(t *testing.T) {
16+
orig := userOutput
17+
defer func() { userOutput = orig }()
18+
19+
var buf bytes.Buffer
20+
userOutput = &buf
21+
notifyUser("To authenticate, visit: %s\n", "https://example.com/auth")
22+
if got := buf.String(); got != "To authenticate, visit: https://example.com/auth\n" {
23+
t.Errorf("unexpected user output: %q", got)
24+
}
25+
26+
userOutput = nil
27+
notifyUser("should not panic")
28+
}
29+
1230
// TestTokenStateMarshaling tests that oauth2.Token can be marshaled/unmarshaled as expected.
1331
func TestTokenStateMarshaling(t *testing.T) {
1432
original := &oauth2.Token{

0 commit comments

Comments
 (0)