diff --git a/packages/smart-signer/lib/consent-page-controller.ts b/packages/smart-signer/lib/consent-page-controller.ts index e540257bf68ae44bb52b1b6be297f1fb96186691..39ad2972d5069e5a6804bfaa783a5578dfeb0c18 100644 --- a/packages/smart-signer/lib/consent-page-controller.ts +++ b/packages/smart-signer/lib/consent-page-controller.ts @@ -4,6 +4,7 @@ import { getLogger } from '@ui/lib/logging'; import { getIronSession } from 'iron-session'; import { IronSessionData } from '@smart-signer/types/common'; import { sessionOptions } from './session'; +import { getSafeRedirectUrl } from './redirect-validation'; const logger = getLogger('app'); @@ -74,7 +75,7 @@ export const consentPageController: GetServerSideProps = async (ctx) => { return { props: { oidcClientDetails, - redirectTo: interactionDetails.returnTo + redirectTo: getSafeRedirectUrl(interactionDetails.returnTo) } }; } diff --git a/packages/smart-signer/lib/login-page-controller.ts b/packages/smart-signer/lib/login-page-controller.ts index b554c04b5f45cc4c2528ba8978eb2003a9eaba9b..b313c698df9005646b031c169dfcbaa27dde09f5 100644 --- a/packages/smart-signer/lib/login-page-controller.ts +++ b/packages/smart-signer/lib/login-page-controller.ts @@ -5,6 +5,7 @@ import { getIronSession } from 'iron-session'; import { IronSessionData } from '@smart-signer/types/common'; import { sessionOptions } from './session'; import { siteConfig } from '@hive/ui/config/site'; +import { getSafeRedirectUrl } from './redirect-validation'; const logger = getLogger('app'); @@ -74,7 +75,7 @@ export const loginPageController: GetServerSideProps = async (ctx) => { throw new Error(message); } - return { props: { redirectTo: interactionDetails.returnTo } }; + return { props: { redirectTo: getSafeRedirectUrl(interactionDetails.returnTo) } }; } else { // logger.info('loginPageController: no uid, so we are not in oauth flow'); } diff --git a/packages/smart-signer/lib/redirect-validation.ts b/packages/smart-signer/lib/redirect-validation.ts new file mode 100644 index 0000000000000000000000000000000000000000..b5ab30406ce36c0559426e19bdb6ea51ccd92517 --- /dev/null +++ b/packages/smart-signer/lib/redirect-validation.ts @@ -0,0 +1,64 @@ +import { siteConfig } from '@ui/config/site'; +import { getLogger } from '@ui/lib/logging'; + +const logger = getLogger('app'); + +/** + * Validates that a redirect URL is safe to use in the OIDC flow. + * + * This provides defense-in-depth protection against open redirect attacks. + * While oidc-provider generates returnTo internally, we validate it to + * protect against potential library bugs or unforeseen manipulation vectors. + * + * Valid redirects are: + * - Relative URLs starting with the OIDC URL prefix (e.g., /oidc/auth/...) + * - Absolute URLs with the same origin as the site + * + * @param redirectUrl - The URL to validate + * @returns true if the redirect is safe, false otherwise + */ +export function isValidOidcRedirect(redirectUrl: string | undefined): boolean { + if (!redirectUrl) { + return false; + } + + // Allow relative URLs starting with the OIDC prefix + if (redirectUrl.startsWith(`${siteConfig.oidcUrlPrefix}/`)) { + return true; + } + + // For absolute URLs, verify same origin + try { + const redirect = new URL(redirectUrl, siteConfig.url); + const site = new URL(siteConfig.url); + + if (redirect.origin === site.origin) { + return true; + } + + // Log rejected external redirects for security monitoring + logger.warn( + 'Rejected OIDC redirect to external origin: %s (expected: %s)', + redirect.origin, + site.origin + ); + return false; + } catch (error) { + logger.warn('Invalid OIDC redirect URL: %s', redirectUrl); + return false; + } +} + +/** + * Returns a safe redirect URL or undefined if the URL is invalid. + * This is a convenience wrapper around isValidOidcRedirect. + * + * @param redirectUrl - The URL to validate + * @returns The original URL if valid, undefined otherwise + */ +export function getSafeRedirectUrl(redirectUrl: string | undefined): string | undefined { + if (isValidOidcRedirect(redirectUrl)) { + return redirectUrl; + } + return undefined; +}