From de138c3599085e2df95898e2b61d3c88b3a0454a Mon Sep 17 00:00:00 2001 From: Josh Holt Date: Fri, 1 May 2026 09:12:00 -0400 Subject: [PATCH] feat(ui): UI does not have ability to forward headers that are not authorization Add an allowlist env for forwarding additional headers to the backend from oauth2-proxy Signed-off-by: Josh Holt --- helm/kagent/templates/ui-deployment.yaml | 4 + helm/kagent/values.yaml | 8 + .../[namespace]/[agentName]/route.ts | 8 +- .../app/a2a/[namespace]/[agentName]/route.ts | 8 +- ui/src/app/actions/utils.ts | 2 +- ui/src/lib/__tests__/auth.test.ts | 183 ++++++++++++++++++ ui/src/lib/auth.ts | 107 ++++++++-- 7 files changed, 298 insertions(+), 22 deletions(-) create mode 100644 ui/src/lib/__tests__/auth.test.ts diff --git a/helm/kagent/templates/ui-deployment.yaml b/helm/kagent/templates/ui-deployment.yaml index a5b685b5c..92d0503c7 100644 --- a/helm/kagent/templates/ui-deployment.yaml +++ b/helm/kagent/templates/ui-deployment.yaml @@ -62,6 +62,10 @@ spec: - name: SSO_REDIRECT_PATH value: {{ .Values.ui.auth.ssoRedirectPath | default "/oauth2/start" | quote }} {{- end }} + {{- with .Values.ui.additionalForwardedHeaders }} + - name: KAGENT_ADDITIONAL_FORWARDED_HEADERS + value: {{ join "," . | quote }} + {{- end }} {{- with .Values.ui.env }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 446cc54e7..d292d9ec5 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -269,6 +269,14 @@ ui: # Default: /oauth2/start (oauth2-proxy's authentication start endpoint) ssoRedirectPath: "/oauth2/start" env: {} # Additional configuration key-value pairs for the ui ConfigMap + # -- Additional request headers (beyond Authorization) the UI proxy will forward + # to the backend. Names are case-insensitive. Hop-by-hop headers (Connection, + # Transfer-Encoding, etc.) are silently dropped. + additionalForwardedHeaders: [] + # Example: + # additionalForwardedHeaders: + # - X-Forwarded-User + # - X-Forwarded-Email # -- Pod-level security context for the UI pod. Overrides the global podSecurityContext. # @default -- (uses global podSecurityContext) podSecurityContext: {} diff --git a/ui/src/app/a2a-sandboxes/[namespace]/[agentName]/route.ts b/ui/src/app/a2a-sandboxes/[namespace]/[agentName]/route.ts index c01b22243..238ca7782 100644 --- a/ui/src/app/a2a-sandboxes/[namespace]/[agentName]/route.ts +++ b/ui/src/app/a2a-sandboxes/[namespace]/[agentName]/route.ts @@ -1,6 +1,6 @@ import { NextRequest, NextResponse } from 'next/server'; import { getBackendUrl } from '@/lib/utils'; -import { getAuthHeadersFromRequest } from '@/lib/auth'; +import { getAuthHeadersFromRequest, CORS_ALLOW_HEADERS } from '@/lib/auth'; export async function POST( request: NextRequest, @@ -19,12 +19,12 @@ export async function POST( const backendResponse = await fetch(targetUrl, { method: 'POST', headers: { + ...authHeaders, 'Content-Type': 'application/json', 'Accept': 'text/event-stream', 'Cache-Control': 'no-cache', 'Connection': 'keep-alive', 'User-Agent': 'kagent-ui', - ...authHeaders, }, body: JSON.stringify(a2aRequest), }); @@ -49,7 +49,7 @@ export async function POST( 'Connection': 'keep-alive', 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Accept', + 'Access-Control-Allow-Headers': CORS_ALLOW_HEADERS, }); const KEEP_ALIVE_INTERVAL_MS = 30000; @@ -137,7 +137,7 @@ export async function OPTIONS() { headers: { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Accept', + 'Access-Control-Allow-Headers': CORS_ALLOW_HEADERS, 'Access-Control-Max-Age': '86400', }, }); diff --git a/ui/src/app/a2a/[namespace]/[agentName]/route.ts b/ui/src/app/a2a/[namespace]/[agentName]/route.ts index dd3816335..8f5eb9cc9 100644 --- a/ui/src/app/a2a/[namespace]/[agentName]/route.ts +++ b/ui/src/app/a2a/[namespace]/[agentName]/route.ts @@ -1,6 +1,6 @@ import { NextRequest, NextResponse } from 'next/server'; import { getBackendUrl } from '@/lib/utils'; -import { getAuthHeadersFromRequest } from '@/lib/auth'; +import { getAuthHeadersFromRequest, CORS_ALLOW_HEADERS } from '@/lib/auth'; export async function POST( request: NextRequest, @@ -20,12 +20,12 @@ export async function POST( const backendResponse = await fetch(targetUrl, { method: 'POST', headers: { + ...authHeaders, 'Content-Type': 'application/json', 'Accept': 'text/event-stream', 'Cache-Control': 'no-cache', 'Connection': 'keep-alive', 'User-Agent': 'kagent-ui', - ...authHeaders, }, body: JSON.stringify(a2aRequest), }); @@ -51,7 +51,7 @@ export async function POST( 'Connection': 'keep-alive', 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Accept', + 'Access-Control-Allow-Headers': CORS_ALLOW_HEADERS, }); const KEEP_ALIVE_INTERVAL_MS = 30000; // 30 seconds @@ -142,7 +142,7 @@ export async function OPTIONS() { headers: { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Accept', + 'Access-Control-Allow-Headers': CORS_ALLOW_HEADERS, 'Access-Control-Max-Age': '86400', }, }); diff --git a/ui/src/app/actions/utils.ts b/ui/src/app/actions/utils.ts index ad8a456f2..200a2007b 100644 --- a/ui/src/app/actions/utils.ts +++ b/ui/src/app/actions/utils.ts @@ -25,9 +25,9 @@ export async function fetchApi(path: string, options: ApiOptions = {}): Promi ...options, cache: "no-store", headers: { + ...authHeaders, "Content-Type": "application/json", Accept: "application/json", - ...authHeaders, ...options.headers, }, signal: AbortSignal.timeout(30000), // 30 second timeout diff --git a/ui/src/lib/__tests__/auth.test.ts b/ui/src/lib/__tests__/auth.test.ts new file mode 100644 index 000000000..1f41e456f --- /dev/null +++ b/ui/src/lib/__tests__/auth.test.ts @@ -0,0 +1,183 @@ +import { describe, expect, it, jest, beforeEach, afterAll } from '@jest/globals'; +import { + CORS_ALLOW_HEADERS, + extractAllowedHeaders, + getAuthHeadersFromRequest, + parseAllowedForwardHeaders, +} from '../auth'; +import type { NextRequest } from 'next/server'; + +function fakeRequest(headers: Record): NextRequest { + const lower: Record = {}; + for (const [k, v] of Object.entries(headers)) { + lower[k.toLowerCase()] = v; + } + return { + headers: { + get: (name: string) => lower[name.toLowerCase()] ?? null, + }, + } as unknown as NextRequest; +} + +describe('parseAllowedForwardHeaders', () => { + it('returns just the default forward set when env is undefined', () => { + const { allowed, blocked } = parseAllowedForwardHeaders(undefined); + expect(Array.from(allowed)).toEqual(['authorization']); + expect(blocked).toEqual([]); + }); + + it('returns just the default forward set when env is empty', () => { + const { allowed, blocked } = parseAllowedForwardHeaders(''); + expect(Array.from(allowed)).toEqual(['authorization']); + expect(blocked).toEqual([]); + }); + + it('adds extra headers from env, lowercased', () => { + const { allowed } = parseAllowedForwardHeaders('X-Slack-User, X-Slack-Team'); + expect(allowed.has('authorization')).toBe(true); + expect(allowed.has('x-slack-user')).toBe(true); + expect(allowed.has('x-slack-team')).toBe(true); + }); + + it('trims whitespace and ignores empty entries', () => { + const { allowed } = parseAllowedForwardHeaders(' X-A ,, ,X-B,'); + expect(allowed.has('x-a')).toBe(true); + expect(allowed.has('x-b')).toBe(true); + expect(allowed.size).toBe(3); // authorization + x-a + x-b + }); + + it('drops hop-by-hop / routing headers and reports them as blocked', () => { + const { allowed, blocked } = parseAllowedForwardHeaders( + 'Host, Connection, Transfer-Encoding, Content-Length, X-Slack-User' + ); + expect(allowed.has('host')).toBe(false); + expect(allowed.has('connection')).toBe(false); + expect(allowed.has('transfer-encoding')).toBe(false); + expect(allowed.has('content-length')).toBe(false); + expect(allowed.has('x-slack-user')).toBe(true); + expect(blocked.sort()).toEqual( + ['connection', 'content-length', 'host', 'transfer-encoding'].sort() + ); + }); + + it('treats Authorization listed in env as a no-op (already in default set)', () => { + const { allowed, blocked } = parseAllowedForwardHeaders('Authorization'); + expect(Array.from(allowed)).toEqual(['authorization']); + expect(blocked).toEqual([]); + }); +}); + +describe('extractAllowedHeaders', () => { + it('returns only headers present in the allowlist', () => { + const got = extractAllowedHeaders( + new Set(['authorization', 'x-slack-user']), + (name) => + ({ + authorization: 'Bearer abc', + 'x-slack-user': 'U123', + 'x-other': 'should-not-appear', + } as Record)[name] ?? null + ); + expect(got).toEqual({ + authorization: 'Bearer abc', + 'x-slack-user': 'U123', + }); + }); + + it('skips headers that are absent from the request', () => { + const got = extractAllowedHeaders( + new Set(['authorization', 'x-slack-user']), + (name) => (name === 'authorization' ? 'Bearer abc' : null) + ); + expect(got).toEqual({ authorization: 'Bearer abc' }); + }); + + it('skips empty header values', () => { + const got = extractAllowedHeaders( + new Set(['authorization', 'x-slack-user']), + (name) => (name === 'authorization' ? 'Bearer abc' : ''), + ); + expect(got).toEqual({ authorization: 'Bearer abc' }); + }); + + it('returns an empty record when the allowlist is empty', () => { + const got = extractAllowedHeaders(new Set(), () => 'whatever'); + expect(got).toEqual({}); + }); +}); + +describe('getAuthHeadersFromRequest (env-driven)', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + delete process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS; + }); + + afterAll(() => { + process.env = originalEnv; + }); + + it('forwards only Authorization by default', () => { + const got = getAuthHeadersFromRequest( + fakeRequest({ Authorization: 'Bearer x', 'X-Slack-User': 'U123' }) + ); + expect(got).toEqual({ authorization: 'Bearer x' }); + }); + + it('forwards extra headers when env lists them', () => { + process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS = 'X-Slack-User, X-Slack-Team'; + const got = getAuthHeadersFromRequest( + fakeRequest({ + Authorization: 'Bearer x', + 'X-Slack-User': 'U123', + 'X-Slack-Team': 'T456', + 'X-Not-Listed': 'nope', + }) + ); + expect(got).toEqual({ + authorization: 'Bearer x', + 'x-slack-user': 'U123', + 'x-slack-team': 'T456', + }); + }); + + it('reads env on every call (not frozen at module load)', () => { + const req = fakeRequest({ Authorization: 'Bearer x', 'X-Late': 'v' }); + + expect(getAuthHeadersFromRequest(req)).toEqual({ authorization: 'Bearer x' }); + + process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS = 'X-Late'; + expect(getAuthHeadersFromRequest(req)).toEqual({ + authorization: 'Bearer x', + 'x-late': 'v', + }); + + delete process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS; + expect(getAuthHeadersFromRequest(req)).toEqual({ authorization: 'Bearer x' }); + }); + + it('does not forward hop-by-hop headers even if listed', () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS = 'Host, Connection, X-Slack-User'; + const got = getAuthHeadersFromRequest( + fakeRequest({ + Authorization: 'Bearer x', + Host: 'evil.example.com', + Connection: 'close', + 'X-Slack-User': 'U123', + }) + ); + expect(got).toEqual({ + authorization: 'Bearer x', + 'x-slack-user': 'U123', + }); + warnSpy.mockRestore(); + }); +}); + +describe('CORS_ALLOW_HEADERS', () => { + it('does not advertise additional forwarded headers cross-origin', () => { + expect(CORS_ALLOW_HEADERS).toBe('Content-Type, Authorization, Accept'); + }); +}); diff --git a/ui/src/lib/auth.ts b/ui/src/lib/auth.ts index 033cb1d02..aeeb2a456 100644 --- a/ui/src/lib/auth.ts +++ b/ui/src/lib/auth.ts @@ -1,33 +1,114 @@ import { headers } from "next/headers"; import { NextRequest } from "next/server"; +// Headers that must never be forwarded to the backend, regardless of +// KAGENT_ADDITIONAL_FORWARDED_HEADERS configuration. These are hop-by-hop +// (RFC 7230 §6.1) or determined by the upstream fetch. +const BLOCKED_FORWARD_HEADERS = new Set([ + "host", + "connection", + "keep-alive", + "transfer-encoding", + "upgrade", + "te", + "trailer", + "proxy-authenticate", + "proxy-authorization", + "content-length", + "content-encoding", +]); + +// Headers always forwarded. The Authorization header carries the JWT from +// oauth2-proxy and is required for the backend to identify the user. +const DEFAULT_FORWARD_HEADERS = ["authorization"] as const; + /** - * Extract authentication headers from a headers-like object. - * Common implementation used by both server actions and route handlers. + * Parse a comma-separated header allowlist (case-insensitive) and union it + * with the default forward headers. Hop-by-hop / routing headers are dropped + * and returned separately so callers can surface a warning. + * + * Pure function — no env or module state. Exported for unit testing. */ -function extractAuthHeaders(getHeader: (name: string) => string | null): Record { - const authHeaders: Record = {}; +export function parseAllowedForwardHeaders(raw: string | undefined): { + allowed: Set; + blocked: string[]; +} { + const allowed = new Set(DEFAULT_FORWARD_HEADERS); + const blocked: string[] = []; + if (!raw) { + return { allowed, blocked }; + } + for (const part of raw.split(",")) { + const name = part.trim().toLowerCase(); + if (!name) continue; + if (BLOCKED_FORWARD_HEADERS.has(name)) { + blocked.push(name); + continue; + } + allowed.add(name); + } + return { allowed, blocked }; +} + +let warnedBlocked = false; - // Forward Authorization header (JWT token from oauth2-proxy) - const authHeader = getHeader("Authorization"); - if (authHeader) { - authHeaders["Authorization"] = authHeader; +function getAllowedHeadersFromEnv(): Set { + const { allowed, blocked } = parseAllowedForwardHeaders( + process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS + ); + if (blocked.length > 0 && !warnedBlocked) { + warnedBlocked = true; + console.warn( + `KAGENT_ADDITIONAL_FORWARDED_HEADERS contains hop-by-hop or routing headers that will not be forwarded: ${blocked.join(", ")}` + ); } + return allowed; +} - return authHeaders; +/** + * CORS Access-Control-Allow-Headers value for proxy routes. + * Kept intentionally minimal and decoupled from KAGENT_ADDITIONAL_FORWARDED_HEADERS: + * forwarded identity-style headers (e.g. x-auth-request-user) must not be + * accepted from cross-origin browsers, even when the backend trusts them + * server-to-server. + */ +export const CORS_ALLOW_HEADERS = "Content-Type, Authorization, Accept"; + +/** + * Copy headers named in `allowed` from `getHeader` into a forwardable record. + * Pure function — exported for unit testing. + */ +export function extractAllowedHeaders( + allowed: Set, + getHeader: (name: string) => string | null +): Record { + const forwarded: Record = {}; + for (const name of allowed) { + const value = getHeader(name); + if (value) { + forwarded[name] = value; + } + } + return forwarded; +} + +function extractAuthHeaders(getHeader: (name: string) => string | null): Record { + return extractAllowedHeaders(getAllowedHeadersFromEnv(), getHeader); } /** - * Get authentication headers from incoming request (for route handlers). - * These are set by oauth2-proxy or other auth proxies. + * Get authentication and additional forwarded headers from incoming request (for route handlers). + * Always forwards Authorization (set by oauth2-proxy or other auth proxies); extra headers + * may be configured via KAGENT_ADDITIONAL_FORWARDED_HEADERS. */ export function getAuthHeadersFromRequest(request: NextRequest): Record { return extractAuthHeaders((name) => request.headers.get(name)); } /** - * Get authentication headers from request context (for server actions). - * These are set by oauth2-proxy or other auth proxies. + * Get authentication and additional forwarded headers from request context (for server actions). + * Always forwards Authorization (set by oauth2-proxy or other auth proxies); extra headers + * may be configured via KAGENT_ADDITIONAL_FORWARDED_HEADERS. */ export async function getAuthHeadersFromContext(): Promise> { const headersList = await headers();