Skip to content

Commit eee0b8d

Browse files
authored
Relay the caller's Authorization to the agent when fetching its card (#75)
GET /chat/{ns}/{name}/agent-card sent no Authorization header to the agent, so an agent hosted by `authbridge exec` could never have its card fetched: the inbound pipeline answered the unauthenticated request with 401 and WWW-Authenticate: Bearer. `agents card` and `agents chat` both failed, and --with-authorization did not help — that flag attaches the token to the A2A message, which is sent after the card lookup that was failing. Confirmed against a live agent by the pipeline's own policy header: the violation was auth.malformed_header before, meaning no header arrived, and auth.token_expired after, meaning the relayed token arrived and was parsed. A stub agent driven through the real command logs the full bearer token. The header is forwarded verbatim rather than read from the config file. This handler proxies one request and the caller has already chosen which identity to present; reading a token from disk would attach a credential to a request that deliberately carried none, and would make the response depend on state the caller cannot see. The destination is the instance record's loopback inbound address, never anything the request controls, which is what makes relaying a bearer token acceptable here. The 401 hint was also wrong for this case, naming the one remedy that cannot work. A 401 the server relayed from an agent now says the credentials were accepted and that signing in will not help, pointing at the pipeline policy and the token's audience instead. An unrecognized body still gets the original sign-in hint, so nothing regresses. `agents chat` help claimed the card lookup always carries the context's token; it does not when --server is given. The weather-service example comments out its spiffe and mtls blocks, since mTLS requires the SPIFFE block and a local run has no workload API socket. Assisted by Claude. Signed-off-by: Ed Snible <snible@us.ibm.com>
1 parent df778cd commit eee0b8d

6 files changed

Lines changed: 269 additions & 15 deletions

File tree

cmd/agents_chat.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@ perfectly reachable; --address http://<route>:<port> is the way past it.
4040
` + "`a2a send`" + `: the message text is sent as a single user text part, the
4141
response is streamed event by event as it arrives, and --with-authorization
4242
attaches the effective context's bearer token as an Authorization header on each
43-
request. Note that the card lookup always carries the context's token, since it
44-
goes to the platform API, while the message carries one only with
45-
--with-authorization.
43+
request. Note that the card lookup carries the context's token, since it goes to
44+
the platform API — but an explicit --server sends none — while the message carries
45+
one only with --with-authorization. A local cortex relays that token on to the
46+
agent when it fetches the card, which is what lets it describe an agent hosted
47+
behind an authbridge inbound pipeline.
4648
4749
With --verbose both the card lookup and the message are reported on stderr.`,
4850
Args: cobra.ExactArgs(1),

cmd/root.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"fmt"
1313
"net/http"
1414
"os"
15+
"strings"
1516

1617
"github.com/spf13/cobra"
1718

@@ -244,6 +245,14 @@ func errorHint(err error) string {
244245

245246
switch statusErr.StatusCode {
246247
case http.StatusUnauthorized:
248+
// A 401 the server relayed from something it called on our behalf is a
249+
// different problem with different advice: our credentials were accepted, so
250+
// signing in again fixes nothing. Reported separately rather than folded in,
251+
// because the generic hint actively misleads here — it names the one remedy
252+
// that cannot work.
253+
if upstream := relayedUnauthorizedHint(statusErr.Body); upstream != "" {
254+
return upstream
255+
}
247256
// Deliberately not suggested for 403: that is an authenticated
248257
// identity lacking permission, where signing in again changes
249258
// nothing and the advice would send the user in a circle.
@@ -253,6 +262,27 @@ func errorHint(err error) string {
253262
}
254263
}
255264

265+
// relayedUnauthorizedHint returns advice for a 401 whose body shows the server was
266+
// reporting an upstream's refusal rather than its own, or "" when it was not.
267+
//
268+
// The distinction is worth drawing because the two are indistinguishable by status
269+
// alone, and the remedies are opposites: for our own rejection, sign in again; for a
270+
// relayed one, our credentials were fine and the agent is what refused them.
271+
//
272+
// Matched on the body text the agent-card endpoint produces (see
273+
// internal/serve/agentcard.go), which is also the shape the Python backend's
274+
// equivalent returns. A body this does not recognize yields "" and the caller falls
275+
// back to the generic hint, so an unrecognized 401 is no worse than before.
276+
func relayedUnauthorizedHint(body string) string {
277+
if !strings.Contains(body, "failed to fetch agent card from") {
278+
return ""
279+
}
280+
return "Hint: your credentials were accepted; the agent itself refused them, so " +
281+
"`rossoctl login` will not help. An agent hosted by `authbridge exec` sits behind " +
282+
"its inbound pipeline — check that pipeline's policy, and that the token carries " +
283+
"the audience and scopes it requires (`rossoctl auth status`)."
284+
}
285+
256286
func init() {
257287
rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "enable verbose output")
258288
rootCmd.PersistentFlags().StringVar(&server, "server", "", "Rossoctl API server URI (overrides the current context; default: current context's server)")

cmd/root_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,56 @@ func TestErrorHintQuietForOtherErrors(t *testing.T) {
320320
})
321321
}
322322
}
323+
324+
// TestErrorHintDistinguishesRelayedUnauthorized verifies a 401 the server relayed
325+
// from an agent it called on our behalf does not get the sign-in hint.
326+
//
327+
// The two 401s are indistinguishable by status but have opposite remedies. This body
328+
// is what `agents card` produces for an agent behind an `authbridge exec` inbound
329+
// pipeline: our credentials were accepted by the server, the agent refused them, and
330+
// `rossoctl login` cannot help — so naming it would send the user to the one remedy
331+
// that is guaranteed not to work.
332+
func TestErrorHintDistinguishesRelayedUnauthorized(t *testing.T) {
333+
err := &apiclient.StatusError{
334+
Endpoint: "http://localhost:9097/api/v1/chat/team1/weather-praxis/agent-card",
335+
StatusCode: http.StatusUnauthorized,
336+
Body: `{"detail":"failed to fetch agent card from 127.0.0.1:38531: agent returned 401"}`,
337+
}
338+
339+
hint := errorHint(err)
340+
if hint == "" {
341+
t.Fatal("a relayed 401 should still produce a hint")
342+
}
343+
// It must not *instruct* a sign-in. Naming the command to rule it out is fine —
344+
// and is why this checks for the instruction rather than for the command name,
345+
// which appears in "`rossoctl login` will not help".
346+
if strings.Contains(hint, "Run `rossoctl login`") {
347+
t.Errorf("hint %q must not instruct a sign-in: the server accepted the credentials", hint)
348+
}
349+
if !strings.Contains(hint, "will not help") {
350+
t.Errorf("hint %q should say signing in will not help, so the reader stops reaching for it", hint)
351+
}
352+
// It has to say whose refusal this was, or the reader is left where they started.
353+
if !strings.Contains(hint, "agent") {
354+
t.Errorf("hint %q should say the agent refused the credentials", hint)
355+
}
356+
}
357+
358+
// TestErrorHintFallsBackForUnrecognizedBody verifies a 401 whose body is not a
359+
// recognized relay still gets the ordinary sign-in hint.
360+
//
361+
// The relay detection is a body-text match, so it must fail open: an unrecognized
362+
// 401 should be no worse off than before the distinction existed.
363+
func TestErrorHintFallsBackForUnrecognizedBody(t *testing.T) {
364+
for _, body := range []string{
365+
`{"detail":"Token signing key not found"}`,
366+
`{"detail":"failed to connect to agent at 127.0.0.1:38531: connection refused"}`,
367+
"",
368+
} {
369+
err := &apiclient.StatusError{StatusCode: http.StatusUnauthorized, Body: body}
370+
hint := errorHint(err)
371+
if !strings.Contains(hint, "rossoctl login") {
372+
t.Errorf("for body %q, hint = %q; want the sign-in hint", body, hint)
373+
}
374+
}
375+
}

examples/authbridge-local-weather-service.yaml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ listener:
3636
# :8000 — that is authbridge's address, in front of it.
3737
reverse_proxy_backend: http://127.0.0.1:8001
3838
mode: proxy-sidecar
39-
mtls:
40-
mode: permissive
4139
pipeline:
4240
inbound:
4341
plugins:
@@ -61,5 +59,8 @@ pipeline:
6159
keycloak_realm: rossoctl
6260
keycloak_url: http://keycloak.localtest.me:8080/
6361
name: token-exchange
64-
spiffe:
65-
socket: unix:///spiffe-workload-api/spire-agent.sock
62+
# spiffe:
63+
# socket: unix:///spiffe-workload-api/spire-agent.sock
64+
# Commented out, because mTLS requires the SPIFFE block
65+
# mtls:
66+
# mode: permissive

internal/serve/agentcard.go

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ const agentCardPath = "/.well-known/agent-card.json"
2727
// must not hang this server's own client indefinitely.
2828
const agentCardTimeout = 10 * time.Second
2929

30-
// cardFetcher fetches the card document at the given URL. A variable so a test can
31-
// answer without a live agent, following lister and getter above; production
30+
// cardFetcher fetches the card document at the given URL, sending authorization
31+
// verbatim as the Authorization header when it is non-empty. A variable so a test
32+
// can answer without a live agent, following lister and getter above; production
3233
// always uses the real HTTP client.
3334
var cardFetcher = fetchCardDocument
3435

@@ -84,8 +85,25 @@ func agentCardRoute(opts) http.HandlerFunc {
8485
return
8586
}
8687

88+
// The caller's Authorization header is relayed to the agent. An agent hosted
89+
// by `authbridge exec` sits behind the inbound pipeline, which answers an
90+
// unauthenticated request with 401 and WWW-Authenticate: Bearer — so without
91+
// this, a card could never be fetched for exactly the agents this server
92+
// exists to describe, and the failure surfaced as a 401 from this endpoint
93+
// that read as "your credentials were rejected" when they had never been sent.
94+
//
95+
// Relayed verbatim rather than read from the config file: this handler is a
96+
// proxy for one request, and the caller has already chosen which identity to
97+
// present. Reading a token from disk would make the response depend on state
98+
// the caller cannot see, and would attach a credential to a request that
99+
// deliberately carried none.
100+
//
101+
// Only ever sent to inst.InboundAddr, which is a loopback address this host
102+
// recorded for a process it started. That matters: relaying a bearer token is
103+
// safe here because the destination is not caller-controlled — it comes from
104+
// the instance record, not from the request.
87105
cardURL := "http://" + inst.InboundAddr + agentCardPath
88-
body, status, err := cardFetcher(r.Context(), cardURL)
106+
body, status, err := cardFetcher(r.Context(), cardURL, r.Header.Get("Authorization"))
89107
if err != nil {
90108
// 503, matching the backend's httpx.RequestError branch: the agent could
91109
// not be reached, which is the agent's state and not a fault in this
@@ -125,10 +143,16 @@ func agentCardRoute(opts) http.HandlerFunc {
125143
// fetchCardDocument GETs the card document, returning the body and status. The
126144
// body is read even for an error status so a caller can report what was served.
127145
//
146+
// authorization, when non-empty, is sent as the Authorization header verbatim —
147+
// scheme included, since the caller's own header is being forwarded rather than a
148+
// token being formatted. An empty value sends no header at all, which is what an
149+
// unauthenticated caller asked for and what an agent with no inbound policy
150+
// expects.
151+
//
128152
// The read is capped: this server is asking a process it does not control, and an
129153
// agent streaming an endless body should fail rather than consume this server's
130154
// memory. A card is a few kilobytes, so a megabyte is generous.
131-
func fetchCardDocument(ctx context.Context, cardURL string) ([]byte, int, error) {
155+
func fetchCardDocument(ctx context.Context, cardURL, authorization string) ([]byte, int, error) {
132156
ctx, cancel := context.WithTimeout(ctx, agentCardTimeout)
133157
defer cancel()
134158

@@ -137,6 +161,9 @@ func fetchCardDocument(ctx context.Context, cardURL string) ([]byte, int, error)
137161
return nil, 0, err
138162
}
139163
req.Header.Set("Accept", "application/json")
164+
if authorization != "" {
165+
req.Header.Set("Authorization", authorization)
166+
}
140167

141168
resp, err := http.DefaultClient.Do(req)
142169
if err != nil {

internal/serve/agentcard_test.go

Lines changed: 145 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,24 @@ import (
1313
// stubCardFetcher replaces the card fetch with a canned answer, so these tests do
1414
// not need a live agent listening on the fixture's inbound address.
1515
func stubCardFetcher(t *testing.T, body string, status int, err error) *string {
16+
t.Helper()
17+
requested, _ := stubCardFetcherRecording(t, body, status, err)
18+
return requested
19+
}
20+
21+
// stubCardFetcherRecording is stubCardFetcher, additionally reporting the
22+
// Authorization value the handler passed on. Separate so the existing callers stay
23+
// as they were: only the relay tests care about the header.
24+
func stubCardFetcherRecording(t *testing.T, body string, status int, err error) (requestedURL, authorization *string) {
1625
t.Helper()
1726
saved := cardFetcher
18-
var requested string
19-
cardFetcher = func(_ context.Context, cardURL string) ([]byte, int, error) {
20-
requested = cardURL
27+
var gotURL, gotAuth string
28+
cardFetcher = func(_ context.Context, cardURL, auth string) ([]byte, int, error) {
29+
gotURL, gotAuth = cardURL, auth
2130
return []byte(body), status, err
2231
}
2332
t.Cleanup(func() { cardFetcher = saved })
24-
return &requested
33+
return &gotURL, &gotAuth
2534
}
2635

2736
// a2aCard is a card as a v0.3 A2A server serves one: url at the top level,
@@ -346,3 +355,135 @@ func TestAgentCardWithoutInboundAddressIsUnavailable(t *testing.T) {
346355
t.Errorf("detail = %q, should name the instance", detail)
347356
}
348357
}
358+
359+
// getCardWithAuth requests the endpoint with an Authorization header, returning the
360+
// status. Separate from getCard because these tests care about what was relayed
361+
// rather than about the decoded card.
362+
func getCardWithAuth(t *testing.T, ts *httptest.Server, path, authorization string) int {
363+
t.Helper()
364+
req, err := http.NewRequest(http.MethodGet, ts.URL+path, nil)
365+
if err != nil {
366+
t.Fatalf("building request: %v", err)
367+
}
368+
if authorization != "" {
369+
req.Header.Set("Authorization", authorization)
370+
}
371+
res, err := ts.Client().Do(req)
372+
if err != nil {
373+
t.Fatalf("GET %s: %v", path, err)
374+
}
375+
defer func() { _ = res.Body.Close() }()
376+
return res.StatusCode
377+
}
378+
379+
// TestAgentCardRelaysAuthorization verifies the caller's Authorization header is
380+
// forwarded to the agent.
381+
//
382+
// Without this the card of an agent hosted by `authbridge exec` can never be
383+
// fetched: the inbound pipeline answers an unauthenticated request with 401 and
384+
// WWW-Authenticate: Bearer, and the failure surfaced here as a 401 that read as
385+
// "your credentials were rejected" when none had been sent.
386+
func TestAgentCardRelaysAuthorization(t *testing.T) {
387+
stubGetter(t, mixedInstances())
388+
_, auth := stubCardFetcherRecording(t, a2aCard, http.StatusOK, nil)
389+
ts := newTestServer(t, "/api/v1")
390+
391+
const token = "Bearer test-token-value"
392+
if status := getCardWithAuth(t, ts, "/api/v1/chat/recorded1/swift-falcon-0001/agent-card", token); status != http.StatusOK {
393+
t.Fatalf("status = %d, want 200", status)
394+
}
395+
396+
// Verbatim, scheme included: the caller's own header is being forwarded, not a
397+
// token being reformatted.
398+
if *auth != token {
399+
t.Errorf("relayed Authorization = %q, want %q", *auth, token)
400+
}
401+
}
402+
403+
// TestAgentCardRelaysAuthorizationVerbatim verifies a non-Bearer scheme is passed on
404+
// unchanged, rather than being parsed or reformatted.
405+
//
406+
// The handler does not interpret the credential — an agent's inbound policy decides
407+
// what it accepts, and rewriting the header here would break a scheme this code does
408+
// not know about.
409+
func TestAgentCardRelaysAuthorizationVerbatim(t *testing.T) {
410+
for _, header := range []string{
411+
"Bearer abc.def.ghi",
412+
"Basic dXNlcjpwYXNz",
413+
"DPoP some-other-credential",
414+
} {
415+
t.Run(header, func(t *testing.T) {
416+
stubGetter(t, mixedInstances())
417+
_, auth := stubCardFetcherRecording(t, a2aCard, http.StatusOK, nil)
418+
ts := newTestServer(t, "/api/v1")
419+
420+
getCardWithAuth(t, ts, "/api/v1/chat/recorded1/swift-falcon-0001/agent-card", header)
421+
if *auth != header {
422+
t.Errorf("relayed %q, want %q", *auth, header)
423+
}
424+
})
425+
}
426+
}
427+
428+
// TestAgentCardSendsNoAuthorizationWhenNoneGiven verifies an unauthenticated request
429+
// stays unauthenticated.
430+
//
431+
// The handler must not supply a credential of its own — reading a token from the
432+
// config file would attach one to a request that deliberately carried none, and would
433+
// make the response depend on state the caller cannot see.
434+
func TestAgentCardSendsNoAuthorizationWhenNoneGiven(t *testing.T) {
435+
stubGetter(t, mixedInstances())
436+
_, auth := stubCardFetcherRecording(t, a2aCard, http.StatusOK, nil)
437+
ts := newTestServer(t, "/api/v1")
438+
439+
getCard(t, ts, "/api/v1/chat/recorded1/swift-falcon-0001/agent-card")
440+
441+
if *auth != "" {
442+
t.Errorf("relayed Authorization = %q, want none for an unauthenticated caller", *auth)
443+
}
444+
}
445+
446+
// TestFetchCardDocumentSetsAuthorization verifies the real fetcher — not the stub —
447+
// sends the header, and sends none when the value is empty.
448+
//
449+
// Worth testing against a live server: every test above replaces cardFetcher, so
450+
// nothing else covers the function that actually builds the outbound request.
451+
func TestFetchCardDocumentSetsAuthorization(t *testing.T) {
452+
for _, tc := range []struct {
453+
name string
454+
authorization string
455+
wantHeader string
456+
wantPresent bool
457+
}{
458+
{"with a token", "Bearer xyz", "Bearer xyz", true},
459+
{"empty sends no header", "", "", false},
460+
} {
461+
t.Run(tc.name, func(t *testing.T) {
462+
var got string
463+
var present bool
464+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
465+
got = r.Header.Get("Authorization")
466+
_, present = r.Header["Authorization"]
467+
if r.Header.Get("Accept") != "application/json" {
468+
t.Errorf("Accept = %q, want application/json", r.Header.Get("Accept"))
469+
}
470+
_, _ = w.Write([]byte(`{}`))
471+
}))
472+
defer srv.Close()
473+
474+
_, status, err := fetchCardDocument(context.Background(), srv.URL, tc.authorization)
475+
if err != nil {
476+
t.Fatalf("fetchCardDocument: %v", err)
477+
}
478+
if status != http.StatusOK {
479+
t.Errorf("status = %d, want 200", status)
480+
}
481+
if got != tc.wantHeader {
482+
t.Errorf("Authorization = %q, want %q", got, tc.wantHeader)
483+
}
484+
if present != tc.wantPresent {
485+
t.Errorf("Authorization present = %v, want %v", present, tc.wantPresent)
486+
}
487+
})
488+
}
489+
}

0 commit comments

Comments
 (0)