diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl index f6a6d9f73..63ec26a8e 100644 --- a/helm/kagent/templates/_helpers.tpl +++ b/helm/kagent/templates/_helpers.tpl @@ -299,3 +299,28 @@ imagePullSecrets: {{- toYaml $global | nindent 2 }} {{- end -}} {{- end -}} + +{{/* +Body of oauth2-proxy's custom sign_in.html template (see +templates/oauth2-proxy-templates.yaml). Kept as its own named template, rather +than inline in that ConfigMap, so oauth2-proxy.extraEnv in values.yaml can hash +the content. + +oauth2-proxy renders this as its own Go html/template (not a Helm template) when +it shows the sign-in page to an unauthenticated visitor -- e.g. a request to +/agents/foo is served this page at /oauth2/sign_in?rd=%2Fagents%2Ffoo. +`Redirect` is oauth2-proxy's template variable carrying that original +destination (escaped with a Helm string-literal action so Helm emits it for +oauth2-proxy to evaluate, instead of trying to evaluate it itself). It is +forwarded to kagent's branded /login page. +*/}} +{{- define "kagent.oauth2ProxySignInHTML" -}} + + + + + + +Redirecting to login... + +{{- end -}} diff --git a/helm/kagent/templates/oauth2-proxy-templates.yaml b/helm/kagent/templates/oauth2-proxy-templates.yaml index 0223c9d21..70397c527 100644 --- a/helm/kagent/templates/oauth2-proxy-templates.yaml +++ b/helm/kagent/templates/oauth2-proxy-templates.yaml @@ -7,13 +7,9 @@ metadata: labels: {{- include "kagent.labels" . | nindent 4 }} data: + # The body lives in the kagent.oauth2ProxySignInHTML named template + # (_helpers.tpl) so oauth2-proxy.extraEnv in values.yaml can hash the content to + # force a rollout when it changes. sign_in.html: | - - - - - - - Redirecting to login... - + {{- include "kagent.oauth2ProxySignInHTML" . | nindent 4 }} {{- end }} diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 1e2264585..5b4b2f595 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -771,6 +771,11 @@ oauth2-proxy: mountPath: /templates readOnly: true + extraEnv: + # Forces a rollout whenever the sign_in.html ConfigMap's content changes. + - name: KAGENT_OAUTH2_PROXY_SIGNIN_TEMPLATE_CHECKSUM + value: '{{ include "kagent.oauth2ProxySignInHTML" . | sha256sum }}' + # Mount custom CA certificate for TLS verification (if needed) # Add to extraVolumes: # - name: custom-ca-cert diff --git a/ui/src/app/login/page.tsx b/ui/src/app/login/page.tsx index c64316334..1a559e10e 100644 --- a/ui/src/app/login/page.tsx +++ b/ui/src/app/login/page.tsx @@ -1,12 +1,20 @@ import Link from "next/link"; import KagentLogo from "@/components/kagent-logo"; +import { sanitizeRedirect } from "@/lib/loginRedirect"; import { skipToContentLinkClassName } from "@/lib/skipToContent"; import { cn } from "@/lib/utils"; // SSO redirect path - defaults to oauth2-proxy's start endpoint const SSO_REDIRECT_PATH = process.env.SSO_REDIRECT_PATH || "/oauth2/start"; -export default function LoginPage() { +export default async function LoginPage({ + searchParams, +}: { + searchParams: Promise<{ rd?: string }>; +}) { + const { rd } = await searchParams; + const redirectTo = sanitizeRedirect(rd); + return ( <> {/* Preload background image for faster rendering */} @@ -53,7 +61,7 @@ export default function LoginPage() {
{/* Glow ring */} diff --git a/ui/src/lib/__tests__/loginRedirect.test.ts b/ui/src/lib/__tests__/loginRedirect.test.ts new file mode 100644 index 000000000..7dbda3d2c --- /dev/null +++ b/ui/src/lib/__tests__/loginRedirect.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from '@jest/globals'; +import { sanitizeRedirect } from '../loginRedirect'; + +describe('sanitizeRedirect', () => { + it('passes through a same-origin path', () => { + expect(sanitizeRedirect('/agents/foo')).toBe('/agents/foo'); + }); + + it('passes through a same-origin path with a query string', () => { + expect(sanitizeRedirect('/agents/foo?tab=history')).toBe('/agents/foo?tab=history'); + }); + + it('defaults to "/" when undefined', () => { + expect(sanitizeRedirect(undefined)).toBe('/'); + }); + + it('defaults to "/" for an empty string', () => { + expect(sanitizeRedirect('')).toBe('/'); + }); + + it('rejects an absolute URL', () => { + expect(sanitizeRedirect('https://evil.example.com/phish')).toBe('/'); + }); + + it('treats a bare host+path with no leading slash as a relative path segment', () => { + // Matches URL/browser semantics: with no scheme and no leading "/", + // this resolves relative to the current path rather than a new host. + expect(sanitizeRedirect('evil.example.com/phish')).toBe('/evil.example.com/phish'); + }); + + it('rejects a protocol-relative URL', () => { + expect(sanitizeRedirect('//evil.example.com/phish')).toBe('/'); + }); + + it('rejects a backslash-prefixed path some browsers treat as protocol-relative', () => { + expect(sanitizeRedirect('/\\evil.example.com/phish')).toBe('/'); + }); + + it('rejects a tab-smuggled protocol-relative URL (stripped by the URL parser before host resolution)', () => { + expect(sanitizeRedirect('/\t/evil.example.com/phish')).toBe('/'); + }); + + it('rejects a different scheme entirely', () => { + expect(sanitizeRedirect('javascript:alert(1)')).toBe('/'); + }); +}); diff --git a/ui/src/lib/loginRedirect.ts b/ui/src/lib/loginRedirect.ts new file mode 100644 index 000000000..044485595 --- /dev/null +++ b/ui/src/lib/loginRedirect.ts @@ -0,0 +1,24 @@ +// Any fixed placeholder works here -- it's never dereferenced, just used as +// the base for URL parsing so we can tell whether `rd` stayed same-origin. +const SENTINEL_ORIGIN = "http://kagent-login-redirect.invalid"; + +/** + * Validate a post-login redirect target. + * + * Only a same-origin relative path is safe to hand back to oauth2-proxy's + * `rd` parameter: an absolute URL, a protocol-relative `//host/...`, or a + * disguised variant of either (e.g. a backslash or a stripped tab/newline + * that the URL Standard normalizes into one of the above) would let a + * crafted `/login?rd=...` link send an authenticated session off to an + * attacker's site after sign-in. + */ +export function sanitizeRedirect(rd: string | undefined): string { + if (!rd) return "/"; + try { + const url = new URL(rd, SENTINEL_ORIGIN); + if (url.origin !== SENTINEL_ORIGIN) return "/"; + return url.pathname + url.search + url.hash; + } catch { + return "/"; + } +}