fix(auth): strip internal port from OAuth redirects on BYOD domains (#728)
* fix(auth): strip internal port from OAuth redirects on BYOD domains When BYOD custom domains proxy auth requests through the dashboard to PRIVATE_SERVER_URL (e.g. http://cio-api-vymh:10000), Node's URL host setter preserves the upstream port. Better Auth then redirects browsers to https://customer.example:10000/... which fails. - Add oauth-proxy-redirect helpers to clear backend ports explicitly - Rewrite inbound auth request URLs and outbound oauth-proxy-callback Location headers - Send hostname-only X-Forwarded-Host from dashboard proxy when port matches backend Co-authored-by: Rotimi Best <[email protected]> * fix(auth): address Greptile review on oauth port stripping - Only treat PRIVATE_SERVER_URL as an internal backend port - Add test for rewriteAuthRequestUrl when forwarded host still has :10000 Co-authored-by: Rotimi Best <[email protected]> --------- Co-authored-by: Cursor Agent <[email protected]> Co-authored-by: Rotimi Best <[email protected]>
This commit is contained in:
co-authored by
Rotimi Best
Cursor Agent
parent
3eb5e1268d
commit
2fb6b1c477
+29
-11
@@ -6,6 +6,12 @@ import { isPublicCorsPath, sessionCors } from '@api/middlewares/cors';
|
|||||||
import { randomBytes } from 'crypto';
|
import { randomBytes } from 'crypto';
|
||||||
|
|
||||||
import { API_SERVER_URL } from '@api/constants';
|
import { API_SERVER_URL } from '@api/constants';
|
||||||
|
import {
|
||||||
|
resolveOAuthBrowserOrigin,
|
||||||
|
rewriteAuthRequestUrl,
|
||||||
|
rewriteOAuthProxyCallbackLocation,
|
||||||
|
shouldRewriteOAuthProxyCallbackLocation
|
||||||
|
} from '@api/utils/oauth-proxy-redirect';
|
||||||
import { Hono } from '@api/utils/hono';
|
import { Hono } from '@api/utils/hono';
|
||||||
import { ErrorCodes } from '@api/utils/errors';
|
import { ErrorCodes } from '@api/utils/errors';
|
||||||
import { accountRouter } from '@api/routes/account';
|
import { accountRouter } from '@api/routes/account';
|
||||||
@@ -114,13 +120,16 @@ export const app = new Hono()
|
|||||||
let request = c.req.raw;
|
let request = c.req.raw;
|
||||||
const fwdHost = c.req.header('x-forwarded-host');
|
const fwdHost = c.req.header('x-forwarded-host');
|
||||||
const fwdProto = c.req.header('x-forwarded-proto');
|
const fwdProto = c.req.header('x-forwarded-proto');
|
||||||
|
const browserOrigin = resolveOAuthBrowserOrigin({
|
||||||
|
forwardedHost: fwdHost,
|
||||||
|
forwardedProto: fwdProto
|
||||||
|
});
|
||||||
|
|
||||||
if (fwdHost) {
|
if (fwdHost) {
|
||||||
const url = new URL(request.url);
|
request = rewriteAuthRequestUrl(request, {
|
||||||
url.host = fwdHost;
|
forwardedHost: fwdHost,
|
||||||
if (fwdProto === 'https' || fwdProto === 'http') {
|
forwardedProto: fwdProto
|
||||||
url.protocol = `${fwdProto}:`;
|
});
|
||||||
}
|
|
||||||
request = new Request(url, request);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Inbound: if the browser is hitting `/oauth-proxy-callback?token=…`
|
// Inbound: if the browser is hitting `/oauth-proxy-callback?token=…`
|
||||||
@@ -168,8 +177,9 @@ export const app = new Hono()
|
|||||||
// pushes the Location header past Render's response size ceiling and
|
// pushes the Location header past Render's response size ceiling and
|
||||||
// the redirect silently never reaches the browser.
|
// the redirect silently never reaches the browser.
|
||||||
if (response.status >= 300 && response.status < 400) {
|
if (response.status >= 300 && response.status < 400) {
|
||||||
const location = response.headers.get('location');
|
let location = response.headers.get('location');
|
||||||
if (location && location.includes('/oauth-proxy-callback') && location.includes('cookies=')) {
|
|
||||||
|
if (location?.includes('/oauth-proxy-callback') && location.includes('cookies=')) {
|
||||||
try {
|
try {
|
||||||
const outUrl = new URL(location);
|
const outUrl = new URL(location);
|
||||||
const cookies = outUrl.searchParams.get('cookies');
|
const cookies = outUrl.searchParams.get('cookies');
|
||||||
@@ -178,14 +188,22 @@ export const app = new Hono()
|
|||||||
await storeHandoffPayload(token, cookies);
|
await storeHandoffPayload(token, cookies);
|
||||||
outUrl.searchParams.delete('cookies');
|
outUrl.searchParams.delete('cookies');
|
||||||
outUrl.searchParams.set('token', token);
|
outUrl.searchParams.set('token', token);
|
||||||
const headers = new Headers(response.headers);
|
location = outUrl.toString();
|
||||||
headers.set('location', outUrl.toString());
|
|
||||||
return new Response(response.body, { status: response.status, headers });
|
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error('[auth-handler] failed to swap cookies → token, falling back to inline cookies:', error);
|
console.error('[auth-handler] failed to swap cookies → token, falling back to inline cookies:', error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (location && browserOrigin && shouldRewriteOAuthProxyCallbackLocation(location, browserOrigin)) {
|
||||||
|
location = rewriteOAuthProxyCallbackLocation(location, browserOrigin);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (location && location !== response.headers.get('location')) {
|
||||||
|
const headers = new Headers(response.headers);
|
||||||
|
headers.set('location', location);
|
||||||
|
return new Response(response.body, { status: response.status, headers });
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (response.status >= 300 && response.status < 400) {
|
if (response.status >= 300 && response.status < 400) {
|
||||||
|
|||||||
@@ -0,0 +1,86 @@
|
|||||||
|
import { afterEach, describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import {
|
||||||
|
resolveOAuthBrowserOrigin,
|
||||||
|
rewriteAuthRequestUrl,
|
||||||
|
rewriteOAuthProxyCallbackLocation,
|
||||||
|
shouldRewriteOAuthProxyCallbackLocation,
|
||||||
|
stripBackendPortFromHost
|
||||||
|
} from '../oauth-proxy-redirect';
|
||||||
|
|
||||||
|
const ORIGINAL_ENV = { ...process.env };
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
process.env = { ...ORIGINAL_ENV };
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('stripBackendPortFromHost', () => {
|
||||||
|
it('removes a port that matches PRIVATE_SERVER_URL', () => {
|
||||||
|
process.env.PRIVATE_SERVER_URL = 'http://cio-api-vymh:10000';
|
||||||
|
|
||||||
|
expect(stripBackendPortFromHost('my.fastlearner.io:10000')).toBe('my.fastlearner.io');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps non-backend ports such as local dev', () => {
|
||||||
|
process.env.PRIVATE_SERVER_URL = 'http://localhost:3002';
|
||||||
|
|
||||||
|
expect(stripBackendPortFromHost('localhost:5173')).toBe('localhost:5173');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('rewriteAuthRequestUrl', () => {
|
||||||
|
it('clears the internal upstream port from the rewritten request URL', () => {
|
||||||
|
process.env.PRIVATE_SERVER_URL = 'http://cio-api-vymh:10000';
|
||||||
|
|
||||||
|
const request = new Request('http://cio-api-vymh:10000/api/auth/sign-in/social', { method: 'POST' });
|
||||||
|
const rewritten = rewriteAuthRequestUrl(request, {
|
||||||
|
forwardedHost: 'my.fastlearner.io',
|
||||||
|
forwardedProto: 'https'
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(new URL(rewritten.url).href).toBe('https://my.fastlearner.io/api/auth/sign-in/social');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips a backend port from x-forwarded-host before rewriting the request URL', () => {
|
||||||
|
process.env.PRIVATE_SERVER_URL = 'http://cio-api-vymh:10000';
|
||||||
|
|
||||||
|
const request = new Request('http://cio-api-vymh:10000/api/auth/sign-in/social', { method: 'POST' });
|
||||||
|
const rewritten = rewriteAuthRequestUrl(request, {
|
||||||
|
forwardedHost: 'my.fastlearner.io:10000',
|
||||||
|
forwardedProto: 'https'
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(new URL(rewritten.url).href).toBe('https://my.fastlearner.io/api/auth/sign-in/social');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('resolveOAuthBrowserOrigin', () => {
|
||||||
|
it('prefers x-forwarded-host without the backend port', () => {
|
||||||
|
process.env.PRIVATE_SERVER_URL = 'http://cio-api-vymh:10000';
|
||||||
|
|
||||||
|
expect(
|
||||||
|
resolveOAuthBrowserOrigin({
|
||||||
|
forwardedHost: 'my.fastlearner.io:10000',
|
||||||
|
forwardedProto: 'https'
|
||||||
|
})
|
||||||
|
).toBe('https://my.fastlearner.io');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to DASHBOARD_ORIGIN when forwarded host is absent', () => {
|
||||||
|
process.env.DASHBOARD_ORIGIN = 'https://my.fastlearner.io';
|
||||||
|
|
||||||
|
expect(resolveOAuthBrowserOrigin({})).toBe('https://my.fastlearner.io');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('rewriteOAuthProxyCallbackLocation', () => {
|
||||||
|
it('rewrites backend host to the public dashboard origin', () => {
|
||||||
|
const location =
|
||||||
|
'https://my.fastlearner.io:10000/api/auth/oauth-proxy-callback?callbackURL=https%3A%2F%2Fmy.fastlearner.io%2Fcourse%2Ffoo%2Fenroll&token=abc';
|
||||||
|
|
||||||
|
expect(shouldRewriteOAuthProxyCallbackLocation(location, 'https://my.fastlearner.io')).toBe(true);
|
||||||
|
expect(rewriteOAuthProxyCallbackLocation(location, 'https://my.fastlearner.io')).toBe(
|
||||||
|
'https://my.fastlearner.io/api/auth/oauth-proxy-callback?callbackURL=https%3A%2F%2Fmy.fastlearner.io%2Fcourse%2Ffoo%2Fenroll&token=abc'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,118 @@
|
|||||||
|
/**
|
||||||
|
* Better Auth's `oAuthProxy` plugin redirects the browser to
|
||||||
|
* `{host}/api/auth/oauth-proxy-callback` on the host that initiated OAuth.
|
||||||
|
*
|
||||||
|
* BYOD custom domains reach the API through the dashboard's internal proxy
|
||||||
|
* (`PRIVATE_SERVER_URL`, often `http://service:10000`). Node's URL setter keeps
|
||||||
|
* the upstream port unless it is cleared explicitly — e.g. `url.host =
|
||||||
|
* 'my.fastlearner.io'` on a `http://cio-api-vymh:10000/...` request still
|
||||||
|
* yields `https://my.fastlearner.io:10000/...`.
|
||||||
|
*/
|
||||||
|
|
||||||
|
function readBackendPorts(): Set<number> {
|
||||||
|
const ports = new Set<number>();
|
||||||
|
const privateServerUrl = process.env.PRIVATE_SERVER_URL?.trim();
|
||||||
|
|
||||||
|
if (!privateServerUrl) {
|
||||||
|
return ports;
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
const parsed = new URL(privateServerUrl);
|
||||||
|
if (parsed.port) {
|
||||||
|
ports.add(Number(parsed.port));
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Ignore malformed env values.
|
||||||
|
}
|
||||||
|
|
||||||
|
return ports;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Strip a backend/API port when it appears on a browser-facing host header. */
|
||||||
|
export function stripBackendPortFromHost(host: string): string {
|
||||||
|
const [hostname, portValue] = host.includes(':') ? host.split(':', 2) : [host, ''];
|
||||||
|
if (!portValue || !hostname) return host;
|
||||||
|
|
||||||
|
const port = Number(portValue);
|
||||||
|
if (!Number.isFinite(port) || !readBackendPorts().has(port)) {
|
||||||
|
return host;
|
||||||
|
}
|
||||||
|
|
||||||
|
return hostname;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Public origin where the browser should complete the oauth-proxy handoff. */
|
||||||
|
export function resolveOAuthBrowserOrigin(options: {
|
||||||
|
forwardedHost?: string | null;
|
||||||
|
forwardedProto?: string | null;
|
||||||
|
}): string | null {
|
||||||
|
const dashboardOrigin = process.env.DASHBOARD_ORIGIN?.trim().replace(/\/$/, '');
|
||||||
|
|
||||||
|
if (options.forwardedHost) {
|
||||||
|
const proto =
|
||||||
|
options.forwardedProto === 'http' || options.forwardedProto === 'https' ? options.forwardedProto : 'https';
|
||||||
|
const host = stripBackendPortFromHost(options.forwardedHost);
|
||||||
|
|
||||||
|
return `${proto}://${host}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (dashboardOrigin) {
|
||||||
|
return dashboardOrigin;
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Rebuild the auth request URL from proxy headers so Better Auth sees the
|
||||||
|
* browser-facing host without the internal upstream port.
|
||||||
|
*/
|
||||||
|
export function rewriteAuthRequestUrl(
|
||||||
|
request: Request,
|
||||||
|
options: { forwardedHost?: string | null; forwardedProto?: string | null }
|
||||||
|
): Request {
|
||||||
|
const { forwardedHost, forwardedProto } = options;
|
||||||
|
if (!forwardedHost?.trim()) {
|
||||||
|
return request;
|
||||||
|
}
|
||||||
|
|
||||||
|
const url = new URL(request.url);
|
||||||
|
const publicHost = stripBackendPortFromHost(forwardedHost.trim());
|
||||||
|
const [hostname, explicitPort = ''] = publicHost.includes(':') ? publicHost.split(':', 2) : [publicHost, ''];
|
||||||
|
|
||||||
|
url.hostname = hostname!;
|
||||||
|
url.port = explicitPort;
|
||||||
|
|
||||||
|
if (forwardedProto === 'https' || forwardedProto === 'http') {
|
||||||
|
url.protocol = `${forwardedProto}:`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return new Request(url, request);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function shouldRewriteOAuthProxyCallbackLocation(location: string, browserOrigin: string): boolean {
|
||||||
|
if (!location.includes('/oauth-proxy-callback')) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
const target = new URL(location);
|
||||||
|
const origin = new URL(browserOrigin.endsWith('/') ? browserOrigin : `${browserOrigin}/`);
|
||||||
|
|
||||||
|
return target.host !== origin.host || target.protocol !== origin.protocol;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export function rewriteOAuthProxyCallbackLocation(location: string, browserOrigin: string): string {
|
||||||
|
const target = new URL(location);
|
||||||
|
const origin = new URL(browserOrigin.endsWith('/') ? browserOrigin : `${browserOrigin}/`);
|
||||||
|
|
||||||
|
target.protocol = origin.protocol;
|
||||||
|
target.hostname = origin.hostname;
|
||||||
|
target.port = origin.port;
|
||||||
|
|
||||||
|
return target.toString();
|
||||||
|
}
|
||||||
@@ -34,8 +34,27 @@ function resolveApiUpstreamBase(): string | null {
|
|||||||
return base.replace(/\/$/, '');
|
return base.replace(/\/$/, '');
|
||||||
}
|
}
|
||||||
|
|
||||||
function resolveOriginalHost(request: Request, url: URL): string {
|
function resolveBrowserForwardedHost(request: Request, url: URL): string {
|
||||||
return request.headers.get('x-forwarded-host') ?? request.headers.get('host') ?? url.host;
|
const raw = request.headers.get('x-forwarded-host') ?? request.headers.get('host') ?? url.hostname;
|
||||||
|
const [hostname, port = ''] = raw.includes(':') ? raw.split(':', 2) : [raw, ''];
|
||||||
|
|
||||||
|
if (!port) {
|
||||||
|
return hostname;
|
||||||
|
}
|
||||||
|
|
||||||
|
const privateServerUrl = process.env.PRIVATE_SERVER_URL;
|
||||||
|
if (privateServerUrl) {
|
||||||
|
try {
|
||||||
|
const backendPort = new URL(privateServerUrl).port;
|
||||||
|
if (backendPort && port === backendPort) {
|
||||||
|
return hostname;
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Ignore malformed PRIVATE_SERVER_URL.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return raw;
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function proxyRequestToApi(request: Request): Promise<Response> {
|
export async function proxyRequestToApi(request: Request): Promise<Response> {
|
||||||
@@ -49,7 +68,7 @@ export async function proxyRequestToApi(request: Request): Promise<Response> {
|
|||||||
|
|
||||||
const upstreamPath = resolveApiUpstreamPath(url.pathname);
|
const upstreamPath = resolveApiUpstreamPath(url.pathname);
|
||||||
const upstreamUrl = new URL(`${upstreamPath}${url.search}`, apiBase);
|
const upstreamUrl = new URL(`${upstreamPath}${url.search}`, apiBase);
|
||||||
const originalHost = resolveOriginalHost(request, url);
|
const originalHost = resolveBrowserForwardedHost(request, url);
|
||||||
|
|
||||||
const upstreamHeaders = new Headers(request.headers);
|
const upstreamHeaders = new Headers(request.headers);
|
||||||
upstreamHeaders.set('host', upstreamUrl.host);
|
upstreamHeaders.set('host', upstreamUrl.host);
|
||||||
|
|||||||
Reference in New Issue
Block a user