Skip to content

Commit

Permalink
Enforce configured host on user supplied returnTo (#557)
Browse files Browse the repository at this point in the history
* Enforce configured host on user supplied returnTo

* Test improvements per PR suggestions
  • Loading branch information
adamjmcgrath committed Dec 15, 2021
1 parent 129650d commit 0bbd9f8
Show file tree
Hide file tree
Showing 6 changed files with 571 additions and 33 deletions.
17 changes: 10 additions & 7 deletions src/handlers/login.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { NextApiResponse, NextApiRequest } from 'next';
import { AuthorizationParameters, HandleLogin as BaseHandleLogin } from '../auth0-session';
import isSafeRedirect from '../utils/url-helpers';
import toSafeRedirect from '../utils/url-helpers';
import { assertReqRes } from '../utils/assert';
import { NextConfig } from '../config';
import { BaseConfig, NextConfig } from '../config';
import { HandlerError } from '../utils/errors';

/**
Expand Down Expand Up @@ -117,16 +117,19 @@ export type HandleLogin = (req: NextApiRequest, res: NextApiResponse, options?:
/**
* @ignore
*/
export default function handleLoginFactory(handler: BaseHandleLogin, nextConfig: NextConfig): HandleLogin {
export default function handleLoginFactory(
handler: BaseHandleLogin,
nextConfig: NextConfig,
baseConfig: BaseConfig
): HandleLogin {
return async (req, res, options = {}): Promise<void> => {
try {
assertReqRes(req, res);
if (req.query.returnTo) {
const returnTo = Array.isArray(req.query.returnTo) ? req.query.returnTo[0] : req.query.returnTo;
const dangerousReturnTo = Array.isArray(req.query.returnTo) ? req.query.returnTo[0] : req.query.returnTo;
const safeBaseUrl = new URL(options.authorizationParams?.redirect_uri || baseConfig.baseURL);

if (!isSafeRedirect(returnTo)) {
throw new Error('Invalid value provided for returnTo, must be a relative url');
}
const returnTo = toSafeRedirect(dangerousReturnTo, safeBaseUrl);

options = { ...options, returnTo };
}
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const initAuth0: InitAuth0 = (params) => {
const getAccessToken = accessTokenFactory(nextConfig, getClient, sessionCache);
const withApiAuthRequired = withApiAuthRequiredFactory(sessionCache);
const withPageAuthRequired = withPageAuthRequiredFactory(nextConfig.routes.login, getSession);
const handleLogin = loginHandler(baseHandleLogin, nextConfig);
const handleLogin = loginHandler(baseHandleLogin, nextConfig, baseConfig);
const handleLogout = logoutHandler(baseHandleLogout);
const handleCallback = callbackHandler(baseHandleCallback, nextConfig);
const handleProfile = profileHandler(getClient, getAccessToken, sessionCache);
Expand Down
21 changes: 11 additions & 10 deletions src/utils/url-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
/**
* Helper which tests if a URL can safely be redirected to. Requires the URL to be relative.
* @param url
* @param dangerousRedirect
* @param safeBaseUrl
*/
export default function isSafeRedirect(url: string): boolean {
if (typeof url !== 'string') {
throw new TypeError(`Invalid url: ${url}`);
export default function toSafeRedirect(dangerousRedirect: string, safeBaseUrl: URL): string | undefined {
let url: URL;
try {
url = new URL(dangerousRedirect, safeBaseUrl);
} catch (e) {
return undefined;
}

// Prevent open redirects using the //foo.com format (double forward slash).
if (/\/\//.test(url)) {
return false;
if (url.origin === safeBaseUrl.origin) {
return url.toString();
}

return !/^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(url);
return undefined;
}

0 comments on commit 0bbd9f8

Please sign in to comment.