From d778ff7e891bc1a259ca952ac3d7c405f75880c1 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 9 Jan 2026 13:36:23 -0800 Subject: [PATCH 1/4] Add external URL redirect whitelist for nextRequest Allow nextRequest parameter to redirect to external URLs on whitelisted domains after login. This enables cross-subdomain redirects (e.g., from narrative.kbase.us to hub.berdl.kbase.us) while maintaining security. Features: - Configurable whitelist via redirect_whitelist in config.json - Wildcard support (e.g., *.berdl.kbase.us) - HTTPS-only enforcement - TLD-only wildcards rejected for security (e.g., *.com blocked) - Blocked redirects show toast and fall back to /narratives --- .env | 3 ++ config.json | 3 +- scripts/build_deploy.ts | 3 ++ src/common/utils/redirectValidation.ts | 70 ++++++++++++++++++++++++++ src/features/login/LogIn.tsx | 35 +++++++++---- 5 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 src/common/utils/redirectValidation.ts diff --git a/.env b/.env index 1ad1e2a5..73a8da83 100644 --- a/.env +++ b/.env @@ -15,5 +15,8 @@ REACT_APP_KBASE_LEGACY_DOMAIN=legacy.ci-europa.kbase.us REACT_APP_KBASE_BACKUP_COOKIE_NAME = 'test_kbase_backup_session' REACT_APP_KBASE_BACKUP_COOKIE_DOMAIN = 'localhost' +# Comma-separated list of allowed external redirect domains (supports wildcards like *.berdl.kbase.us) +REACT_APP_REDIRECT_WHITELIST = '' + EXTEND_ESLINT=true SKIP_PREFLIGHT_CHECK=true diff --git a/config.json b/config.json index 2f929817..70b54adb 100644 --- a/config.json +++ b/config.json @@ -47,7 +47,8 @@ "name": "kbase_session_backup", "domain": ".kbase.us" }, - "cdm_domain": "hub.berdl.kbase.us" + "cdm_domain": "hub.berdl.kbase.us", + "redirect_whitelist": ["*.berdl.kbase.us"] } } } diff --git a/scripts/build_deploy.ts b/scripts/build_deploy.ts index 077afd27..ac4c54c8 100755 --- a/scripts/build_deploy.ts +++ b/scripts/build_deploy.ts @@ -19,6 +19,7 @@ interface EnvironmentConfig { domain: string; }; cdm_domain?: string; + redirect_whitelist?: string[]; } interface BuildParameters { @@ -54,6 +55,7 @@ const setEnvironment = ( public_url: publicURL, backup_cookie: backupCookie, cdm_domain: cdmDomain, + redirect_whitelist: redirectWhitelist, } = environmentConfig; const envsNew: Record = { @@ -65,6 +67,7 @@ const setEnvironment = ( REACT_APP_KBASE_BACKUP_COOKIE_NAME: backupCookie?.name || '', REACT_APP_KBASE_BACKUP_COOKIE_DOMAIN: backupCookie?.domain || '', REACT_APP_KBASE_CDM_DOMAIN: cdmDomain || 'cdmhub.' + domain, + REACT_APP_REDIRECT_WHITELIST: redirectWhitelist?.join(',') || '', }; Object.assign(process.env, envsNew); }; diff --git a/src/common/utils/redirectValidation.ts b/src/common/utils/redirectValidation.ts new file mode 100644 index 00000000..0bb48c75 --- /dev/null +++ b/src/common/utils/redirectValidation.ts @@ -0,0 +1,70 @@ +/** + * Utilities for validating external redirect URLs. + * + * External URLs must be HTTPS and match a whitelisted domain pattern. + * Wildcards like *.berdl.kbase.us are supported, but TLD-only + * wildcards (*.com, *.us) are rejected for security. + */ + +export const getRedirectWhitelist = (): string[] => { + const whitelist = process.env.REACT_APP_REDIRECT_WHITELIST || ''; + return whitelist + .split(',') + .map((d) => d.trim()) + .filter(Boolean); +}; + +/** + * Validates that a wildcard pattern is not too broad. + * Rejects patterns like *.com or *.co.uk that would match any domain. + * Requires at least 2 domain parts after the wildcard. + */ +export const isValidWildcardPattern = (pattern: string): boolean => { + if (!pattern.startsWith('*.')) return true; // exact domain, always valid + const withoutWildcard = pattern.slice(2); + const parts = withoutWildcard.split('.'); + return parts.length >= 2; // e.g., *.berdl.kbase.us has 3 parts +}; + +/** + * Checks if a hostname matches a domain pattern. + * Supports wildcards: *.berdl.kbase.us matches hub.berdl.kbase.us + */ +export const matchesWildcard = (hostname: string, pattern: string): boolean => { + if (pattern.startsWith('*.')) { + const suffix = pattern.slice(1); // ".berdl.kbase.us" + return hostname.endsWith(suffix) || hostname === pattern.slice(2); + } + return hostname === pattern; +}; + +/** + * Validates if a URL is whitelisted for external redirect. + * - Must be HTTPS + * - Must match a pattern in the whitelist + * - Wildcard patterns must not be too broad + */ +export const isWhitelistedExternalUrl = (url: string): boolean => { + try { + const parsed = new URL(url); + + // Must be HTTPS + if (parsed.protocol !== 'https:') return false; + + const whitelist = getRedirectWhitelist(); + return whitelist.some( + (pattern) => + isValidWildcardPattern(pattern) && + matchesWildcard(parsed.hostname, pattern) + ); + } catch { + return false; + } +}; + +/** + * Checks if a string looks like an external URL (starts with http:// or https://) + */ +export const isExternalUrl = (value: string): boolean => { + return value.startsWith('https://') || value.startsWith('http://'); +}; diff --git a/src/features/login/LogIn.tsx b/src/features/login/LogIn.tsx index 5742da15..18c6e56b 100644 --- a/src/features/login/LogIn.tsx +++ b/src/features/login/LogIn.tsx @@ -22,23 +22,40 @@ import { useCookie } from '../../common/cookie'; import { usePageTitle } from '../layout/layoutSlice'; import { ProviderButtons } from '../auth/providers'; import { makeAuthFlowURLs } from '../auth/utils'; +import { + isExternalUrl, + isWhitelistedExternalUrl, +} from '../../common/utils/redirectValidation'; export const useCheckLoggedIn = (nextRequest: string | undefined) => { const { initialized, token } = useAppSelector((state) => state.auth); - const navigate = useNavigate(); + useEffect(() => { - if (token && initialized) { - if (nextRequest) { - try { - const next = JSON.parse(nextRequest) as To; - navigate(next); - } catch { - throw TypeError('nextRequest param cannot be parsed'); - } + if (!token || !initialized) return; + + if (!nextRequest) { + navigate('/narratives'); + return; + } + + // External URLs must be validated against whitelist + if (isExternalUrl(nextRequest)) { + if (isWhitelistedExternalUrl(nextRequest)) { + window.location.href = nextRequest; } else { + console.error('External redirect blocked: URL not in whitelist'); // eslint-disable-line no-console + toast('Redirect blocked: destination not allowed'); navigate('/narratives'); } + return; + } + + // Internal paths are JSON-encoded + try { + navigate(JSON.parse(nextRequest) as To); + } catch { + throw TypeError('nextRequest param cannot be parsed'); } }, [initialized, navigate, nextRequest, token]); }; From bb42042903a231f564bcf4558084235fd648e072 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 9 Jan 2026 14:12:38 -0800 Subject: [PATCH 2/4] Add tests for redirect validation utilities --- src/common/utils/redirectValidation.test.ts | 183 ++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 src/common/utils/redirectValidation.test.ts diff --git a/src/common/utils/redirectValidation.test.ts b/src/common/utils/redirectValidation.test.ts new file mode 100644 index 00000000..9e6536c3 --- /dev/null +++ b/src/common/utils/redirectValidation.test.ts @@ -0,0 +1,183 @@ +import { + isExternalUrl, + isValidWildcardPattern, + matchesWildcard, + getRedirectWhitelist, + isWhitelistedExternalUrl, +} from './redirectValidation'; + +describe('isExternalUrl', () => { + test('returns true for https URLs', () => { + expect(isExternalUrl('https://example.com')).toBe(true); + expect(isExternalUrl('https://hub.berdl.kbase.us/path')).toBe(true); + }); + + test('returns true for http URLs', () => { + expect(isExternalUrl('http://example.com')).toBe(true); + }); + + test('returns false for JSON-encoded paths', () => { + expect(isExternalUrl('{"pathname":"/narratives"}')).toBe(false); + expect(isExternalUrl('"/profile"')).toBe(false); + }); + + test('returns false for plain paths', () => { + expect(isExternalUrl('/narratives')).toBe(false); + expect(isExternalUrl('narratives')).toBe(false); + }); +}); + +describe('isValidWildcardPattern', () => { + test('accepts exact domains', () => { + expect(isValidWildcardPattern('hub.berdl.kbase.us')).toBe(true); + expect(isValidWildcardPattern('example.com')).toBe(true); + }); + + test('accepts wildcards with 2+ domain parts', () => { + expect(isValidWildcardPattern('*.berdl.kbase.us')).toBe(true); + expect(isValidWildcardPattern('*.kbase.us')).toBe(true); + expect(isValidWildcardPattern('*.example.com')).toBe(true); + }); + + test('rejects TLD-only wildcards', () => { + expect(isValidWildcardPattern('*.com')).toBe(false); + expect(isValidWildcardPattern('*.us')).toBe(false); + expect(isValidWildcardPattern('*.org')).toBe(false); + }); +}); + +describe('matchesWildcard', () => { + test('matches exact domains', () => { + expect(matchesWildcard('example.com', 'example.com')).toBe(true); + expect(matchesWildcard('hub.berdl.kbase.us', 'hub.berdl.kbase.us')).toBe( + true + ); + }); + + test('does not match different exact domains', () => { + expect(matchesWildcard('other.com', 'example.com')).toBe(false); + }); + + test('matches wildcard subdomains', () => { + expect(matchesWildcard('hub.berdl.kbase.us', '*.berdl.kbase.us')).toBe( + true + ); + expect(matchesWildcard('hub.dev.berdl.kbase.us', '*.berdl.kbase.us')).toBe( + true + ); + expect(matchesWildcard('anything.kbase.us', '*.kbase.us')).toBe(true); + }); + + test('matches wildcard base domain', () => { + expect(matchesWildcard('berdl.kbase.us', '*.berdl.kbase.us')).toBe(true); + }); + + test('does not match unrelated domains with wildcard', () => { + expect(matchesWildcard('hub.other.com', '*.berdl.kbase.us')).toBe(false); + expect(matchesWildcard('kbase.us', '*.berdl.kbase.us')).toBe(false); + }); +}); + +describe('getRedirectWhitelist', () => { + const originalEnv = process.env.REACT_APP_REDIRECT_WHITELIST; + + afterEach(() => { + process.env.REACT_APP_REDIRECT_WHITELIST = originalEnv; + }); + + test('returns empty array when not set', () => { + delete process.env.REACT_APP_REDIRECT_WHITELIST; + expect(getRedirectWhitelist()).toEqual([]); + }); + + test('returns empty array for empty string', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = ''; + expect(getRedirectWhitelist()).toEqual([]); + }); + + test('parses single domain', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*.berdl.kbase.us'; + expect(getRedirectWhitelist()).toEqual(['*.berdl.kbase.us']); + }); + + test('parses comma-separated domains', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = + '*.berdl.kbase.us,*.other.kbase.us'; + expect(getRedirectWhitelist()).toEqual([ + '*.berdl.kbase.us', + '*.other.kbase.us', + ]); + }); + + test('trims whitespace', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = + ' *.berdl.kbase.us , *.other.kbase.us '; + expect(getRedirectWhitelist()).toEqual([ + '*.berdl.kbase.us', + '*.other.kbase.us', + ]); + }); + + test('filters empty entries', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = + '*.berdl.kbase.us,,*.other.kbase.us,'; + expect(getRedirectWhitelist()).toEqual([ + '*.berdl.kbase.us', + '*.other.kbase.us', + ]); + }); +}); + +describe('isWhitelistedExternalUrl', () => { + const originalEnv = process.env.REACT_APP_REDIRECT_WHITELIST; + + afterEach(() => { + process.env.REACT_APP_REDIRECT_WHITELIST = originalEnv; + }); + + test('returns false when whitelist is empty', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = ''; + expect(isWhitelistedExternalUrl('https://hub.berdl.kbase.us')).toBe(false); + }); + + test('returns true for whitelisted domain', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*.berdl.kbase.us'; + expect(isWhitelistedExternalUrl('https://hub.berdl.kbase.us')).toBe(true); + expect(isWhitelistedExternalUrl('https://hub.berdl.kbase.us/path')).toBe( + true + ); + expect( + isWhitelistedExternalUrl('https://hub.dev.berdl.kbase.us/path?query=1') + ).toBe(true); + }); + + test('returns false for non-whitelisted domain', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*.berdl.kbase.us'; + expect(isWhitelistedExternalUrl('https://evil.com')).toBe(false); + expect(isWhitelistedExternalUrl('https://other.kbase.us')).toBe(false); + }); + + test('returns false for HTTP URLs', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*.berdl.kbase.us'; + expect(isWhitelistedExternalUrl('http://hub.berdl.kbase.us')).toBe(false); + }); + + test('returns false for invalid URLs', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*.berdl.kbase.us'; + expect(isWhitelistedExternalUrl('not-a-url')).toBe(false); + expect(isWhitelistedExternalUrl('')).toBe(false); + }); + + test('returns false when pattern is too broad', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*.com'; + expect(isWhitelistedExternalUrl('https://evil.com')).toBe(false); + }); + + test('works with multiple whitelist entries', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = + '*.berdl.kbase.us,exact.example.com'; + expect(isWhitelistedExternalUrl('https://hub.berdl.kbase.us')).toBe(true); + expect(isWhitelistedExternalUrl('https://exact.example.com')).toBe(true); + expect(isWhitelistedExternalUrl('https://other.example.com')).toBe(false); + }); +}); From d9766dceff21bb01d09190e28c09374814282b8c Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Wed, 14 Jan 2026 16:16:46 -0800 Subject: [PATCH 3/4] Make isExternalUrl case-insensitive for protocol matching Handles case variations like HTTPS://, HTTP://, Https:// to prevent potential issues with non-lowercase URL schemes. --- src/common/utils/redirectValidation.test.ts | 6 ++++++ src/common/utils/redirectValidation.ts | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/common/utils/redirectValidation.test.ts b/src/common/utils/redirectValidation.test.ts index 9e6536c3..cf1cd497 100644 --- a/src/common/utils/redirectValidation.test.ts +++ b/src/common/utils/redirectValidation.test.ts @@ -16,6 +16,12 @@ describe('isExternalUrl', () => { expect(isExternalUrl('http://example.com')).toBe(true); }); + test('handles case variations in protocol', () => { + expect(isExternalUrl('HTTPS://example.com')).toBe(true); + expect(isExternalUrl('HTTP://example.com')).toBe(true); + expect(isExternalUrl('Https://example.com')).toBe(true); + }); + test('returns false for JSON-encoded paths', () => { expect(isExternalUrl('{"pathname":"/narratives"}')).toBe(false); expect(isExternalUrl('"/profile"')).toBe(false); diff --git a/src/common/utils/redirectValidation.ts b/src/common/utils/redirectValidation.ts index 0bb48c75..d4c3ab2a 100644 --- a/src/common/utils/redirectValidation.ts +++ b/src/common/utils/redirectValidation.ts @@ -66,5 +66,6 @@ export const isWhitelistedExternalUrl = (url: string): boolean => { * Checks if a string looks like an external URL (starts with http:// or https://) */ export const isExternalUrl = (value: string): boolean => { - return value.startsWith('https://') || value.startsWith('http://'); + const lower = value.toLowerCase(); + return lower.startsWith('https://') || lower.startsWith('http://'); }; From 3bb3d545687391d175fb2ec7ef8323d68ee4e8c2 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 15 Jan 2026 10:21:34 -0800 Subject: [PATCH 4/4] Simplify redirect whitelist validation per PR review - Remove isValidWildcardPattern function (provided false sense of security) - Trust whitelist as configured, only reject literal "*" - Add default whitelist value (*.kbase.us) for local development - Clarify .env header comment about config.json overwriting values --- .env | 6 ++--- src/common/utils/redirectValidation.test.ts | 29 +++++---------------- src/common/utils/redirectValidation.ts | 26 +++++------------- 3 files changed, 17 insertions(+), 44 deletions(-) diff --git a/.env b/.env index 73a8da83..bc91f363 100644 --- a/.env +++ b/.env @@ -1,5 +1,5 @@ -# Includes env defaults for local development, -# for deploy enviroment configurations, see config.json +# Defaults for local development only. +# These values are overwritten by config.json when deployed. # Base URL path for the enviroment PUBLIC_URL = '/dev/' @@ -16,7 +16,7 @@ REACT_APP_KBASE_BACKUP_COOKIE_NAME = 'test_kbase_backup_session' REACT_APP_KBASE_BACKUP_COOKIE_DOMAIN = 'localhost' # Comma-separated list of allowed external redirect domains (supports wildcards like *.berdl.kbase.us) -REACT_APP_REDIRECT_WHITELIST = '' +REACT_APP_REDIRECT_WHITELIST = '*.kbase.us' EXTEND_ESLINT=true SKIP_PREFLIGHT_CHECK=true diff --git a/src/common/utils/redirectValidation.test.ts b/src/common/utils/redirectValidation.test.ts index cf1cd497..1947f0de 100644 --- a/src/common/utils/redirectValidation.test.ts +++ b/src/common/utils/redirectValidation.test.ts @@ -1,6 +1,5 @@ import { isExternalUrl, - isValidWildcardPattern, matchesWildcard, getRedirectWhitelist, isWhitelistedExternalUrl, @@ -33,25 +32,6 @@ describe('isExternalUrl', () => { }); }); -describe('isValidWildcardPattern', () => { - test('accepts exact domains', () => { - expect(isValidWildcardPattern('hub.berdl.kbase.us')).toBe(true); - expect(isValidWildcardPattern('example.com')).toBe(true); - }); - - test('accepts wildcards with 2+ domain parts', () => { - expect(isValidWildcardPattern('*.berdl.kbase.us')).toBe(true); - expect(isValidWildcardPattern('*.kbase.us')).toBe(true); - expect(isValidWildcardPattern('*.example.com')).toBe(true); - }); - - test('rejects TLD-only wildcards', () => { - expect(isValidWildcardPattern('*.com')).toBe(false); - expect(isValidWildcardPattern('*.us')).toBe(false); - expect(isValidWildcardPattern('*.org')).toBe(false); - }); -}); - describe('matchesWildcard', () => { test('matches exact domains', () => { expect(matchesWildcard('example.com', 'example.com')).toBe(true); @@ -174,9 +154,14 @@ describe('isWhitelistedExternalUrl', () => { expect(isWhitelistedExternalUrl('')).toBe(false); }); - test('returns false when pattern is too broad', () => { + test('returns false when whitelist contains literal asterisk', () => { + process.env.REACT_APP_REDIRECT_WHITELIST = '*'; + expect(isWhitelistedExternalUrl('https://anything.com')).toBe(false); + }); + + test('allows broad wildcards like *.com when explicitly configured', () => { process.env.REACT_APP_REDIRECT_WHITELIST = '*.com'; - expect(isWhitelistedExternalUrl('https://evil.com')).toBe(false); + expect(isWhitelistedExternalUrl('https://anything.com')).toBe(true); }); test('works with multiple whitelist entries', () => { diff --git a/src/common/utils/redirectValidation.ts b/src/common/utils/redirectValidation.ts index d4c3ab2a..c0a19877 100644 --- a/src/common/utils/redirectValidation.ts +++ b/src/common/utils/redirectValidation.ts @@ -2,8 +2,7 @@ * Utilities for validating external redirect URLs. * * External URLs must be HTTPS and match a whitelisted domain pattern. - * Wildcards like *.berdl.kbase.us are supported, but TLD-only - * wildcards (*.com, *.us) are rejected for security. + * Wildcards like *.berdl.kbase.us are supported. Literal "*" is rejected. */ export const getRedirectWhitelist = (): string[] => { @@ -14,18 +13,6 @@ export const getRedirectWhitelist = (): string[] => { .filter(Boolean); }; -/** - * Validates that a wildcard pattern is not too broad. - * Rejects patterns like *.com or *.co.uk that would match any domain. - * Requires at least 2 domain parts after the wildcard. - */ -export const isValidWildcardPattern = (pattern: string): boolean => { - if (!pattern.startsWith('*.')) return true; // exact domain, always valid - const withoutWildcard = pattern.slice(2); - const parts = withoutWildcard.split('.'); - return parts.length >= 2; // e.g., *.berdl.kbase.us has 3 parts -}; - /** * Checks if a hostname matches a domain pattern. * Supports wildcards: *.berdl.kbase.us matches hub.berdl.kbase.us @@ -42,7 +29,7 @@ export const matchesWildcard = (hostname: string, pattern: string): boolean => { * Validates if a URL is whitelisted for external redirect. * - Must be HTTPS * - Must match a pattern in the whitelist - * - Wildcard patterns must not be too broad + * - Rejects if whitelist contains literal "*" */ export const isWhitelistedExternalUrl = (url: string): boolean => { try { @@ -52,10 +39,11 @@ export const isWhitelistedExternalUrl = (url: string): boolean => { if (parsed.protocol !== 'https:') return false; const whitelist = getRedirectWhitelist(); - return whitelist.some( - (pattern) => - isValidWildcardPattern(pattern) && - matchesWildcard(parsed.hostname, pattern) + + if (whitelist.includes('*')) return false; + + return whitelist.some((pattern) => + matchesWildcard(parsed.hostname, pattern) ); } catch { return false;