diff --git a/pods/authProviders/src/__tests__/cookieDomain.test.ts b/pods/authProviders/src/__tests__/cookieDomain.test.ts new file mode 100644 index 00000000000..4ab3567def6 --- /dev/null +++ b/pods/authProviders/src/__tests__/cookieDomain.test.ts @@ -0,0 +1,55 @@ +// +// Copyright © 2024 Hardcore Engineering, Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// +import { isValidCookieDomain } from '../cookieDomain' + +describe('isValidCookieDomain', () => { + it('accepts a leading-dot parent domain', () => { + expect(isValidCookieDomain('.example.com')).toBe(true) + }) + + it('accepts a bare parent domain', () => { + expect(isValidCookieDomain('example.com')).toBe(true) + }) + + it('accepts nested subdomains', () => { + expect(isValidCookieDomain('.app.example.com')).toBe(true) + expect(isValidCookieDomain('dev.example.co.uk')).toBe(true) + }) + + it('accepts hyphenated labels', () => { + expect(isValidCookieDomain('.my-app.example.com')).toBe(true) + }) + + it('rejects a single-label TLD like "com"', () => { + expect(isValidCookieDomain('com')).toBe(false) + expect(isValidCookieDomain('.com')).toBe(false) + }) + + it('rejects values with whitespace', () => { + expect(isValidCookieDomain(' example.com')).toBe(false) + expect(isValidCookieDomain('example.com ')).toBe(false) + expect(isValidCookieDomain('exa mple.com')).toBe(false) + }) + + it('rejects protocol or port', () => { + expect(isValidCookieDomain('https://example.com')).toBe(false) + expect(isValidCookieDomain('example.com:8080')).toBe(false) + }) + + it('rejects empty and trailing-dot values', () => { + expect(isValidCookieDomain('')).toBe(false) + expect(isValidCookieDomain('example.')).toBe(false) + }) +}) diff --git a/pods/authProviders/src/cookieDomain.ts b/pods/authProviders/src/cookieDomain.ts new file mode 100644 index 00000000000..2a2ad203667 --- /dev/null +++ b/pods/authProviders/src/cookieDomain.ts @@ -0,0 +1,28 @@ +// +// Copyright © 2024 Hardcore Engineering, Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +/** + * Validates a `SESSION_COOKIE_DOMAIN` value before it is used to widen the + * session cookie scope across subdomains. + * + * Accepts only plausible parent-domain values: an optional leading dot + * followed by at least two dot-separated labels ending in a 2+ character TLD + * (e.g. `.example.com`, `example.com`). Rejects whitespace, protocol, port + * and single-label values like `com` — the latter would span the cookie + * across a whole TLD and is a common misconfiguration footgun. + */ +export function isValidCookieDomain (domain: string): boolean { + return /^\.?([a-z0-9-]+\.)+[a-z]{2,}$/i.test(domain) +} diff --git a/pods/authProviders/src/index.ts b/pods/authProviders/src/index.ts index 40d7a83c7ba..b4ecf17dca8 100644 --- a/pods/authProviders/src/index.ts +++ b/pods/authProviders/src/index.ts @@ -2,6 +2,7 @@ import Koa from 'koa' import passport from 'koa-passport' import Router from 'koa-router' import session from 'koa-session' +import { isValidCookieDomain } from './cookieDomain' import { registerGithub } from './github' import { registerGoogle } from './google' import { registerOpenid } from './openid' @@ -45,7 +46,41 @@ export function registerProviders ( } app.keys = [serverSecret] - app.use(session({}, app)) + // koa-session defaults the cookie domain to the request host, which + // breaks OIDC flows that start on one subdomain (e.g. dev.example.com) + // and get the callback on another (e.g. app.example.com) — the session + // cookie set on dev isn't sent back to app, so the Passport state lookup + // fails with "did not find expected authorization request details in + // session". Setting SESSION_COOKIE_DOMAIN=.example.com makes the cookie + // span both hosts so the flow completes. Empty / unset preserves prior + // behaviour. + // + // Only set SESSION_COOKIE_DOMAIN when (a) all subdomains are equally + // trusted and (b) HTTPS is terminated in front of this service: widening + // the cookie scope forces secure=true + sameSite=lax below, so the + // signed httpOnly session cookie (carrying OIDC state/nonce/PKCE verifier) + // is never sent over plain HTTP nor cross-site. An invalid value (e.g. + // "com", or anything with whitespace/protocol/port) is ignored — the + // option falls back to prior behaviour (cookie scoped to the request host). + const sessionCookieDomain = process.env.SESSION_COOKIE_DOMAIN?.trim() + const sessionOpts: Partial = {} + if (sessionCookieDomain !== undefined && sessionCookieDomain.length > 0) { + // Accept only plausible parent-domain values: must contain a dot and be + // free of whitespace/protocol/port. Prevents misconfiguration like "com" + // (which would span the cookie across a whole TLD). + if (!isValidCookieDomain(sessionCookieDomain)) { + ctx.warn('SESSION_COOKIE_DOMAIN ignored: not a valid domain', { value: sessionCookieDomain }) + } else { + sessionOpts.domain = sessionCookieDomain + // A cross-subdomain cookie MUST be hardened once its scope reaches + // beyond the request host (koa-session defaults: secure=false, + // sameSite unset). sameSite='lax' is enough for the top-level GET + // OIDC callback. + sessionOpts.secure = true + sessionOpts.sameSite = 'lax' + } + } + app.use(session(sessionOpts, app)) app.use(passport.initialize()) app.use(passport.session())