From 199ef46ee188c151865da005334bc2a809659a22 Mon Sep 17 00:00:00 2001 From: Aarnav Tale Date: Thu, 4 Dec 2025 10:34:48 -0500 Subject: [PATCH] fix: correctly passthrough and handle a new OIDC redirect_uri system --- CHANGELOG.md | 1 + app/routes/auth/oidc-callback.ts | 33 ++++++++------------- app/routes/auth/oidc-start.ts | 45 ++++++++++++++--------------- app/server/config/config-schema.ts | 29 ++++++++++++++++++- app/server/index.ts | 1 + app/server/web/oidc-connector.ts | 11 +++++-- app/utils/oidc-state.ts | 46 ++++++++++++++++++++++++++++++ config.example.yaml | 5 ++++ 8 files changed, 124 insertions(+), 47 deletions(-) create mode 100644 app/utils/oidc-state.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 792bae3..0db51e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Secret path loading has been reworked from the ground up to be more reliable (closes [#334](https://github.com/tale/headplane/issues/334)). - Added better testing and validation for configuration loading - Re-worked the OIDC integration to adhere to the correct standards and surface more errors to the user. + - Deprecated `oidc.redirect_uri` and automated callback URL detection in favor of setting `server.base_url` correctly. - Removed several unnecessarily verbose or spammy log messages. - Updated the minimum Docker API used to support the latest Docker versions (via [#370](https://github.com/tale/headplane/pull/370)). - Enhanced the node tag dialog to show a dropdown of assignable tags (via [#362](https://github.com/tale/headplane/pull/362)). diff --git a/app/routes/auth/oidc-callback.ts b/app/routes/auth/oidc-callback.ts index 36c8593..db6a606 100644 --- a/app/routes/auth/oidc-callback.ts +++ b/app/routes/auth/oidc-callback.ts @@ -1,18 +1,13 @@ import { createHash } from 'node:crypto'; import { count, eq } from 'drizzle-orm'; import * as oidc from 'openid-client'; -import { - createCookie, - data, - type LoaderFunctionArgs, - redirect, -} from 'react-router'; +import { data, type LoaderFunctionArgs, redirect } from 'react-router'; import { ulid } from 'ulidx'; import type { LoadContext } from '~/server'; import { users } from '~/server/db/schema'; import { Roles } from '~/server/web/roles'; import log from '~/utils/log'; -import type { OidcCookieState } from './oidc-start'; +import { createOidcStateCookie } from '~/utils/oidc-state'; export async function loader({ request, @@ -27,32 +22,28 @@ export async function loader({ return redirect('/login?s=error_no_query'); } - const cookie = createCookie('__oidc_auth_flow', { - httpOnly: true, - maxAge: 300, - secure: context.config.server.cookie_secure, - domain: context.config.server.cookie_domain, - }); + const cookie = createOidcStateCookie(context.config); + const oidcCookieState = await cookie.parse(request.headers.get('Cookie')); - const oidcCookieState: OidcCookieState | null = await cookie.parse( - request.headers.get('Cookie'), - ); - - if (oidcCookieState == null || typeof oidcCookieState !== 'object') { + if (oidcCookieState == null) { log.warn('auth', 'Called OIDC callback without session cookie'); return redirect('/login?s=error_no_session'); } - const { state, nonce } = oidcCookieState; - if (!state || !nonce) { + const { state, nonce, redirect_uri } = oidcCookieState; + if (!state || !nonce || !redirect_uri) { log.warn('auth', 'OIDC session cookie is missing required fields'); return redirect('/login?s=error_invalid_session'); } try { + const callbackUrl = new URL(redirect_uri); + const currentUrl = new URL(request.url); + callbackUrl.search = currentUrl.search; + const tokens = await oidc.authorizationCodeGrant( context.oidcConnector.client, - request, + callbackUrl, { expectedState: state, expectedNonce: nonce, diff --git a/app/routes/auth/oidc-start.ts b/app/routes/auth/oidc-start.ts index 01947a2..6e996f1 100644 --- a/app/routes/auth/oidc-start.ts +++ b/app/routes/auth/oidc-start.ts @@ -1,16 +1,8 @@ import * as oidc from 'openid-client'; -import { - createCookie, - data, - type LoaderFunctionArgs, - redirect, -} from 'react-router'; +import { data, type LoaderFunctionArgs, redirect } from 'react-router'; import type { LoadContext } from '~/server'; - -export interface OidcCookieState { - nonce: string; - state: string; -} +import { HeadplaneConfig } from '~/server/config/config-schema'; +import { createOidcStateCookie } from '~/utils/oidc-state'; export async function loader({ request, @@ -25,15 +17,8 @@ export async function loader({ throw data('OIDC is not enabled or misconfigured', { status: 501 }); } - const cookie = createCookie('__oidc_auth_flow', { - httpOnly: true, - maxAge: 300, - secure: context.config.server.cookie_secure, - domain: context.config.server.cookie_domain, - }); - - const redirectUri = - context.config.oidc?.redirect_uri ?? getRedirectUri(request); + const cookie = createOidcStateCookie(context.config); + const redirect_uri = getRedirectUri(context.config, request); const nonce = oidc.randomNonce(); const state = oidc.randomState(); @@ -41,7 +26,7 @@ export async function loader({ const url = oidc.buildAuthorizationUrl(context.oidcConnector.client, { ...(context.oidcConnector.extraParams ?? {}), scope: context.oidcConnector.scope, - redirect_uri: redirectUri, + redirect_uri, state, nonce, }); @@ -52,12 +37,26 @@ export async function loader({ 'Set-Cookie': await cookie.serialize({ state, nonce, - } satisfies OidcCookieState), + redirect_uri, + }), }, }); } -function getRedirectUri(req: Request) { +function getRedirectUri(config: HeadplaneConfig, req: Request): string { + if (config.server.base_url != null) { + const url = new URL(`${__PREFIX__}/oidc/callback`, config.server.base_url); + return url.href; + } + + if (config.oidc?.redirect_uri != null) { + const url = new URL( + `${__PREFIX__}/oidc/callback`, + config.oidc.redirect_uri, + ); + return url.href; + } + const url = new URL(`${__PREFIX__}/oidc/callback`, req.url); let host = req.headers.get('Host'); if (!host) { diff --git a/app/server/config/config-schema.ts b/app/server/config/config-schema.ts index 38ce95d..1b786bc 100644 --- a/app/server/config/config-schema.ts +++ b/app/server/config/config-schema.ts @@ -1,4 +1,5 @@ import { type } from 'arktype'; +import log from '~/utils/log'; import DockerIntegration from './integration/docker'; import KubernetesIntegration from './integration/kubernetes'; import ProcIntegration from './integration/proc'; @@ -14,6 +15,7 @@ export const pathSupportedKeys = [ const serverConfig = type({ host: 'string.ip = "0.0.0.0"', port: 'number.integer = 3000', + base_url: 'string.url?', data_path: 'string.lower = "/var/lib/headplane/"', cookie_secret: '(32 <= string <= 32)', @@ -25,6 +27,7 @@ const serverConfig = type({ const partialServerConfig = type({ host: 'string.ip?', port: 'number.integer?', + base_url: 'string.url?', data_path: 'string.lower?', cookie_secret: '(32 <= string <= 32)?', @@ -62,7 +65,31 @@ const oidcConfig = type({ client_id: 'string', client_secret: 'string', headscale_api_key: 'string', - redirect_uri: 'string.url?', + redirect_uri: type('string.url') + .pipe((value, ctx) => { + log.warn( + 'config', + '%s is deprecated and will be removed in 0.7.0', + ctx.propString, + ); + + const cleanedValue = new URL(value.trim()); + if (cleanedValue.pathname.endsWith(`${__PREFIX__}/oidc/callback`)) { + cleanedValue.pathname = cleanedValue.pathname.replace( + `${__PREFIX__}/oidc/callback`, + '/', + ); + + log.warn( + 'config', + 'Please migrate to using `server.base_url` with a value of "%s"', + cleanedValue.toString(), + ); + } + + return cleanedValue.toString(); + }) + .optional(), disable_api_key_login: 'boolean = false', scope: 'string = "openid email profile"', profile_picture_source: '"oidc" | "gravatar" = "oidc"', diff --git a/app/server/index.ts b/app/server/index.ts index 20142cc..58bb329 100644 --- a/app/server/index.ts +++ b/app/server/index.ts @@ -83,6 +83,7 @@ const appLoadContext = { integration: await loadIntegration(config.integration), oidcConnector: config.oidc ? await createOidcConnector( + config.server.base_url, config.oidc, hsApi.getRuntimeClient(config.oidc.headscale_api_key), ) diff --git a/app/server/web/oidc-connector.ts b/app/server/web/oidc-connector.ts index 1f9d2fb..15b0a60 100644 --- a/app/server/web/oidc-connector.ts +++ b/app/server/web/oidc-connector.ts @@ -40,16 +40,23 @@ export type OidcConnector = * Creates an OIDC connector based on the configuration and Headscale API. * This will attempt to validate the configuration and return any errors. * + * @param baseUrl The base URL of the Headplane server. * @param config The OIDC configuration. * @param client The Headscale runtime API client. * @returns An OIDC connector with validation status. */ export async function createOidcConnector( + baseUrl: string | undefined, config: OidcConfig, client: RuntimeApiClient, ): Promise { - // TODO: MEANINGFUL LOGS NOT JUST DEBUG SPAM LOL - // + if (baseUrl == null && config.redirect_uri == null) { + log.warn( + 'config', + 'OIDC is enabled but `server.base_url` is not set in the config. Starting in Headplane 0.7.0 this will be required for OIDC to function properly and will throw errors if not set, see https://headplane.net/features/sso#configuring-oidc for more information.', + ); + } + const errors: OidcConnectorError[] = []; if (!config.headscale_api_key) { errors.push('INVALID_API_KEY'); diff --git a/app/utils/oidc-state.ts b/app/utils/oidc-state.ts new file mode 100644 index 0000000..0ece90c --- /dev/null +++ b/app/utils/oidc-state.ts @@ -0,0 +1,46 @@ +import { createCookie } from 'react-router'; +import type { HeadplaneConfig } from '~/server/config/config-schema'; + +export interface OidcStateCookie { + nonce: string; + state: string; + redirect_uri: string; +} + +export function createOidcStateCookie(config: HeadplaneConfig) { + const cookie = createCookie('__oidc_state', { + httpOnly: true, + maxAge: 1800, + secure: config.server.cookie_secure, + domain: config.server.cookie_domain, + path: `${__PREFIX__}/oidc/callback`, + }); + + return { + ...cookie, + serialize: async (value: OidcStateCookie): Promise => { + return cookie.serialize(value); + }, + + parse: async ( + cookieHeader: string | null, + ): Promise => { + const parsed = await cookie.parse(cookieHeader); + if ( + parsed == null || + typeof parsed !== 'object' || + typeof parsed.nonce !== 'string' || + typeof parsed.state !== 'string' || + typeof parsed.redirect_uri !== 'string' + ) { + return null; + } + + return { + nonce: parsed.nonce, + state: parsed.state, + redirect_uri: parsed.redirect_uri, + }; + }, + }; +} diff --git a/config.example.yaml b/config.example.yaml index 60b140d..970be9d 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -4,6 +4,11 @@ server: host: "0.0.0.0" port: 3000 + # The base URL for Headplane. Please keep in mind that this will be required + # for Headscale to properly function going forward AND it should not include + # the dashboard prefix (/admin) portion. + base_url: "http://localhost:3000" + # The secret used to encode and decode web sessions (must be 32 characters) # You may also provide `cookie_secret_path` instead to read a value from disk. # See https://headplane.net/configuration/#sensitive-values