Fix URL Redirection in OAuth2/OpenID/SAML (directus#21238)

Co-authored-by: Pascal Jufer <[email protected]>
Co-authored-by: Azri Kahar <[email protected]>

.changeset/calm-tables-destroy.md

@@ -0,0 +1,6 @@
+---
+"@directus/api": patch
+"@directus/env": patch
+---
+
+Fixed potential Open Redirect vulnerability with OAuth2/OpenID/SAML SSO providers (via Directus redirect query parameter), by requiring redirect URLs to be enabled via allow list

api/src/auth/drivers/oauth2.ts

@@ -2,6 +2,7 @@ import { useEnv } from '@directus/env';
 import {
 	ErrorCode,
 	InvalidCredentialsError,
+	InvalidPayloadError,
 	InvalidProviderConfigError,
 	InvalidProviderError,
 	InvalidTokenError,
@@ -16,6 +17,7 @@ import jwt from 'jsonwebtoken';
 import type { Client } from 'openid-client';
 import { errors, generators, Issuer } from 'openid-client';
 import { getAuthProvider } from '../../auth.js';
+import { REFRESH_COOKIE_OPTIONS, SESSION_COOKIE_OPTIONS } from '../../constants.js';
 import getDatabase from '../../database/index.js';
 import emitter from '../../emitter.js';
 import { useLogger } from '../../logger.js';
@@ -26,9 +28,9 @@ import type { AuthData, AuthDriverOptions, User } from '../../types/index.js';
 import asyncHandler from '../../utils/async-handler.js';
 import { getConfigFromEnv } from '../../utils/get-config-from-env.js';
 import { getIPFromReq } from '../../utils/get-ip-from-req.js';
+import { isLoginRedirectAllowed } from '../../utils/is-login-redirect-allowed.js';
 import { Url } from '../../utils/url.js';
 import { LocalAuthDriver } from './local.js';
-import { REFRESH_COOKIE_OPTIONS, SESSION_COOKIE_OPTIONS } from '../../constants.js';
 
 export class OAuth2AuthDriver extends LocalAuthDriver {
 	client: Client;
@@ -298,15 +300,16 @@ export function createOAuth2AuthRouter(providerName: string): Router {
 			const provider = getAuthProvider(providerName) as OAuth2AuthDriver;
 			const codeVerifier = provider.generateCodeVerifier();
 			const prompt = !!req.query['prompt'];
+			const redirect = req.query['redirect'];
 
-			const token = jwt.sign(
-				{ verifier: codeVerifier, redirect: req.query['redirect'], prompt },
-				env['SECRET'] as string,
-				{
-					expiresIn: '5m',
-					issuer: 'directus',
-				},
-			);
+			if (isLoginRedirectAllowed(redirect, providerName) === false) {
+				throw new InvalidPayloadError({ reason: `URL "${redirect}" can't be used to redirect after login` });
+			}
+
+			const token = jwt.sign({ verifier: codeVerifier, redirect, prompt }, env['SECRET'] as string, {
+				expiresIn: '5m',
+				issuer: 'directus',
+			});
 
 			res.cookie(`oauth2.${providerName}`, token, {
 				httpOnly: true,

api/src/auth/drivers/openid.ts

@@ -2,6 +2,7 @@ import { useEnv } from '@directus/env';
 import {
 	ErrorCode,
 	InvalidCredentialsError,
+	InvalidPayloadError,
 	InvalidProviderConfigError,
 	InvalidProviderError,
 	InvalidTokenError,
@@ -16,6 +17,7 @@ import jwt from 'jsonwebtoken';
 import type { Client } from 'openid-client';
 import { errors, generators, Issuer } from 'openid-client';
 import { getAuthProvider } from '../../auth.js';
+import { REFRESH_COOKIE_OPTIONS, SESSION_COOKIE_OPTIONS } from '../../constants.js';
 import getDatabase from '../../database/index.js';
 import emitter from '../../emitter.js';
 import { useLogger } from '../../logger.js';
@@ -26,9 +28,9 @@ import type { AuthData, AuthDriverOptions, User } from '../../types/index.js';
 import asyncHandler from '../../utils/async-handler.js';
 import { getConfigFromEnv } from '../../utils/get-config-from-env.js';
 import { getIPFromReq } from '../../utils/get-ip-from-req.js';
+import { isLoginRedirectAllowed } from '../../utils/is-login-redirect-allowed.js';
 import { Url } from '../../utils/url.js';
 import { LocalAuthDriver } from './local.js';
-import { REFRESH_COOKIE_OPTIONS, SESSION_COOKIE_OPTIONS } from '../../constants.js';
 
 export class OpenIDAuthDriver extends LocalAuthDriver {
 	client: Promise<Client>;
@@ -320,7 +322,6 @@ const handleError = (e: any) => {
 
 export function createOpenIDAuthRouter(providerName: string): Router {
 	const env = useEnv();
-
 	const router = Router();
 
 	router.get(
@@ -329,15 +330,16 @@ export function createOpenIDAuthRouter(providerName: string): Router {
 			const provider = getAuthProvider(providerName) as OpenIDAuthDriver;
 			const codeVerifier = provider.generateCodeVerifier();
 			const prompt = !!req.query['prompt'];
+			const redirect = req.query['redirect'];
 
-			const token = jwt.sign(
-				{ verifier: codeVerifier, redirect: req.query['redirect'], prompt },
-				env['SECRET'] as string,
-				{
-					expiresIn: '5m',
-					issuer: 'directus',
-				},
-			);
+			if (isLoginRedirectAllowed(redirect, providerName) === false) {
+				throw new InvalidPayloadError({ reason: `URL "${redirect}" can't be used to redirect after login` });
+			}
+
+			const token = jwt.sign({ verifier: codeVerifier, redirect, prompt }, env['SECRET'] as string, {
+				expiresIn: '5m',
+				issuer: 'directus',
+			});
 
 			res.cookie(`openid.${providerName}`, token, {
 				httpOnly: true,

api/src/auth/drivers/saml.ts

@@ -1,6 +1,12 @@
 import * as validator from '@authenio/samlify-node-xmllint';
 import { useEnv } from '@directus/env';
-import { ErrorCode, InvalidCredentialsError, InvalidProviderError, isDirectusError } from '@directus/errors';
+import {
+	ErrorCode,
+	InvalidCredentialsError,
+	InvalidPayloadError,
+	InvalidProviderError,
+	isDirectusError,
+} from '@directus/errors';
 import express, { Router } from 'express';
 import * as samlify from 'samlify';
 import { getAuthProvider } from '../../auth.js';
@@ -15,6 +21,7 @@ import type { AuthDriverOptions, User } from '../../types/index.js';
 import asyncHandler from '../../utils/async-handler.js';
 import { getConfigFromEnv } from '../../utils/get-config-from-env.js';
 import { LocalAuthDriver } from './local.js';
+import { isLoginRedirectAllowed } from '../../utils/is-login-redirect-allowed.js';
 
 // Register the samlify schema validator
 samlify.setSchemaValidator(validator);
@@ -126,7 +133,13 @@ export function createSAMLAuthRouter(providerName: string) {
 			const parsedUrl = new URL(url);
 
 			if (req.query['redirect']) {
-				parsedUrl.searchParams.append('RelayState', req.query['redirect'] as string);
+				const redirect = req.query['redirect'] as string;
+
+				if (isLoginRedirectAllowed(redirect, providerName) === false) {
+					throw new InvalidPayloadError({ reason: `URL "${redirect}" can't be used to redirect after login` });
+				}
+
+				parsedUrl.searchParams.append('RelayState', redirect);
 			}
 
 			return res.redirect(parsedUrl.toString());

api/src/utils/is-login-redirect-allowed.test.ts

@@ -0,0 +1,78 @@
+import { vi, expect, test, afterEach } from 'vitest';
+import { useEnv } from '@directus/env';
+import { isLoginRedirectAllowed } from './is-login-redirect-allowed.js';
+
+vi.mock('@directus/env');
+
+afterEach(() => {
+	vi.clearAllMocks();
+});
+
+test('isLoginRedirectAllowed returns true with no redirect', () => {
+	const redirect = undefined;
+	const provider = 'local';
+
+	expect(isLoginRedirectAllowed(redirect, provider)).toBe(true);
+});
+
+test('isLoginRedirectAllowed returns false with invalid redirect', () => {
+	const redirect = 123456;
+	const provider = 'local';
+
+	expect(isLoginRedirectAllowed(redirect, provider)).toBe(false);
+});
+
+test('isLoginRedirectAllowed returns true for allowed URL', () => {
+	const provider = 'local';
+
+	vi.mocked(useEnv).mockReturnValue({
+		[`AUTH_${provider.toUpperCase()}_REDIRECT_ALLOW_LIST`]:
+			'http://external.example.com,https://external.example.com,http://external.example.com:8055/test',
+		PUBLIC_URL: 'http://public.example.com',
+	});
+
+	expect(isLoginRedirectAllowed('http://public.example.com', provider)).toBe(true);
+	expect(isLoginRedirectAllowed('http://external.example.com', provider)).toBe(true);
+	expect(isLoginRedirectAllowed('https://external.example.com', provider)).toBe(true);
+	expect(isLoginRedirectAllowed('http://external.example.com:8055/test', provider)).toBe(true);
+});
+
+test('isLoginRedirectAllowed returns false for denied URL', () => {
+	const provider = 'local';
+
+	vi.mocked(useEnv).mockReturnValue({
+		[`AUTH_${provider.toUpperCase()}_REDIRECT_ALLOW_LIST`]: 'http://external.example.com',
+		PUBLIC_URL: 'http://public.example.com',
+	});
+
+	expect(isLoginRedirectAllowed('https://external.example.com', provider)).toBe(false);
+	expect(isLoginRedirectAllowed('http://external.example.com:8055', provider)).toBe(false);
+	expect(isLoginRedirectAllowed('http://external.example.com/test', provider)).toBe(false);
+});
+
+test('isLoginRedirectAllowed returns true for relative paths', () => {
+	const provider = 'local';
+
+	vi.mocked(useEnv).mockReturnValue({
+		[`AUTH_${provider.toUpperCase()}_REDIRECT_ALLOW_LIST`]: 'http://external.example.com',
+		PUBLIC_URL: 'http://public.example.com',
+	});
+
+	expect(isLoginRedirectAllowed('/admin/content', provider)).toBe(true);
+	expect(isLoginRedirectAllowed('../admin/content', provider)).toBe(true);
+	expect(isLoginRedirectAllowed('./admin/content', provider)).toBe(true);
+
+	expect(isLoginRedirectAllowed('http://public.example.com/admin/content', provider)).toBe(true);
+});
+
+test('isLoginRedirectAllowed returns false if missing protocol', () => {
+	const provider = 'local';
+
+	vi.mocked(useEnv).mockReturnValue({
+		[`AUTH_${provider.toUpperCase()}_REDIRECT_ALLOW_LIST`]: 'http://example.com',
+		PUBLIC_URL: 'http://example.com',
+	});
+
+	expect(isLoginRedirectAllowed('//example.com/admin/content', provider)).toBe(false);
+	expect(isLoginRedirectAllowed('//user@password:example.com/', provider)).toBe(false);
+});

api/src/utils/is-login-redirect-allowed.ts

@@ -0,0 +1,41 @@
+import { useEnv } from '@directus/env';
+import { toArray } from '@directus/utils';
+import isUrlAllowed from './is-url-allowed.js';
+
+/**
+ * Checks if the defined redirect after successful SSO login is in the allow list
+ */
+export function isLoginRedirectAllowed(redirect: unknown, provider: string): boolean {
+	if (!redirect) return true; // empty redirect
+	if (typeof redirect !== 'string') return false; // invalid type
+
+	const env = useEnv();
+	const publicUrl = env['PUBLIC_URL'] as string;
+
+	if (URL.canParse(redirect) === false) {
+		if (redirect.startsWith('//') === false) {
+			// should be a relative path like `/admin/test`
+			return true;
+		}
+
+		// domain without protocol `//example.com/test`
+		return false;
+	}
+
+	const { protocol: redirectProtocol, hostname: redirectDomain } = new URL(redirect);
+
+	const envKey = `AUTH_${provider.toUpperCase()}_REDIRECT_ALLOW_LIST`;
+
+	if (envKey in env) {
+		if (isUrlAllowed(redirect, [...toArray(env[envKey] as string), publicUrl])) return true;
+	}
+
+	if (URL.canParse(publicUrl) === false) {
+		return false;
+	}
+
+	// allow redirects to the defined PUBLIC_URL
+	const { protocol: publicProtocol, hostname: publicDomain } = new URL(publicUrl);
+
+	return `${redirectProtocol}//${redirectDomain}` === `${publicProtocol}//${publicDomain}`;
+}

api/src/utils/is-url-allowed.test.ts

@@ -0,0 +1,37 @@
+import { expect, test } from 'vitest';
+import isUrlAllowed from './is-url-allowed.js';
+
+test('isUrlAllowed should allow matching domain', () => {
+	const checkUrl = 'https://directus.io';
+	const allowedUrls = ['https://directus.io/'];
+
+	expect(isUrlAllowed(checkUrl, allowedUrls)).toBe(true);
+});
+
+test('isUrlAllowed should allow matching path', () => {
+	const checkUrl = 'https://directus.io/tv';
+	const allowedUrls = ['https://directus.io/tv'];
+
+	expect(isUrlAllowed(checkUrl, allowedUrls)).toBe(true);
+});
+
+test('isUrlAllowed should block different paths', () => {
+	const checkUrl = 'http://example.com/test1';
+	const allowedUrls = ['http://example.com/test2', 'http://example.com/test3', 'http://example.com/'];
+
+	expect(isUrlAllowed(checkUrl, allowedUrls)).toBe(false);
+});
+
+test('isUrlAllowed should block different domains', () => {
+	const checkUrl = 'http://directus.io/';
+	const allowedUrls = ['http://example.com/', 'http://directus.chat'];
+
+	expect(isUrlAllowed(checkUrl, allowedUrls)).toBe(false);
+});
+
+test('isUrlAllowed blocks varying protocols', () => {
+	const checkUrl = 'http://example.com/';
+	const allowedUrls = ['ftp://example.com/', 'https://example.com/'];
+
+	expect(isUrlAllowed(checkUrl, allowedUrls)).toBe(false);
+});

api/src/utils/is-url-allowed.ts

@@ -3,7 +3,7 @@ import { URL } from 'url';
 import { useLogger } from '../logger.js';
 
 /**
- * Check if url matches allow list either exactly or by domain+path
+ * Check if URL matches allow list either exactly or by origin (protocol+domain+port) + pathname
  */
 export default function isUrlAllowed(url: string, allowList: string | string[]): boolean {
 	const logger = useLogger();
@@ -15,8 +15,8 @@ export default function isUrlAllowed(url: string, allowList: string | string[]):
 	const parsedWhitelist = urlAllowList
 		.map((allowedURL) => {
 			try {
-				const { hostname, pathname } = new URL(allowedURL);
-				return hostname + pathname;
+				const { origin, pathname } = new URL(allowedURL);
+				return origin + pathname;
 			} catch {
 				logger.warn(`Invalid URL used "${url}"`);
 			}
@@ -26,8 +26,8 @@ export default function isUrlAllowed(url: string, allowList: string | string[]):
 		.filter((f) => f) as string[];
 
 	try {
-		const { hostname, pathname } = new URL(url);
-		return parsedWhitelist.includes(hostname + pathname);
+		const { origin, pathname } = new URL(url);
+		return parsedWhitelist.includes(origin + pathname);
 	} catch {
 		return false;
 	}

docs/releases/breaking-changes.md

@@ -134,6 +134,17 @@ AuthenticationService.login('email', 'password', { otp: 'otp-code', session: tru
 
 :::
 
+### Introduced Allow List for OAuth2/OpenID/SAML Redirects
+
+Due to an Open Redirect vulnerability with the OAuth2, OpenID and SAML SSO providers, we have introduced an allow list
+for these redirects.
+
+If your current workflow depends on redirecting to an external domain after successful SSO login using the
+`?redirect=http://example.com/login` query parameter, then you'll need to add this URL to the
+`AUTH_<PROVIDER>_REDIRECT_ALLOW_LIST` config option.
+
+`AUTH_<PROVIDER>_REDIRECT_ALLOW_LIST` accepts a comma-separated list of URLs (path is included in comparison).
+
 ## Version 10.9.0
 
 ### Updated Exif Tags