🐛 fix: fix callback url error during signin period (#11139)

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-03-27 13:29:15 +07:00 · 2026-01-06 16:05:49 +08:00
parent 9caa13776b
commit 3fc69c5ad3
5 changed files with 186 additions and 84 deletions
--- a/packages/utils/src/server/correctOIDCUrl.test.ts
+++ b/packages/utils/src/server/correctOIDCUrl.test.ts
@@ -338,9 +338,8 @@ describe('correctOIDCUrl', () => {
      const originalUrl = new URL('http://localhost:3000/auth/callback');
      const result = correctOIDCUrl(mockRequest, originalUrl);

-      // Should return original URL because example.com:8443 doesn't match configured APP_URL (https://example.com)
-      expect(result).toBe(originalUrl);
-      expect(result.toString()).toBe('http://localhost:3000/auth/callback');
+      // Should fall back to host header because example.com:8443 doesn't match configured APP_URL
+      expect(result.toString()).toBe('https://internal.com:3000/auth/callback');
    });

    it('should not need correction when URL hostname matches actual host', () => {
@@ -358,18 +357,18 @@ describe('correctOIDCUrl', () => {
  });

  describe('Open Redirect protection', () => {
-    it('should prevent redirection to malicious external domains', () => {
+    it('should prevent redirection to malicious external domains via x-forwarded-host', () => {
      (mockRequest.headers.get as any).mockImplementation((header: string) => {
-        if (header === 'host') return 'malicious.com';
+        if (header === 'host') return 'example.com';
+        if (header === 'x-forwarded-host') return 'malicious.com';
        return null;
      });

      const originalUrl = new URL('http://localhost:3000/auth/callback');
      const result = correctOIDCUrl(mockRequest, originalUrl);

-      // Should return original URL and not redirect to malicious.com
-      expect(result).toBe(originalUrl);
-      expect(result.toString()).toBe('http://localhost:3000/auth/callback');
+      // Should fall back to host header and not redirect to malicious.com
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
    });

    it('should allow redirection to configured domain (example.com)', () => {
@@ -410,9 +409,9 @@ describe('correctOIDCUrl', () => {
      const originalUrl = new URL('http://localhost:3000/auth/callback');
      const result = correctOIDCUrl(mockRequest, originalUrl);

-      // Should return original URL and not redirect to evil.com
-      expect(result).toBe(originalUrl);
-      expect(result.toString()).toBe('http://localhost:3000/auth/callback');
+      // Should fall back to request host (example.com) and not redirect to evil.com
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
+      expect(result.hostname).not.toBe('evil.com');
    });

    it('should allow localhost in development environment', () => {
@@ -437,30 +436,92 @@ describe('correctOIDCUrl', () => {
      delete process.env.APP_URL;

      (mockRequest.headers.get as any).mockImplementation((header: string) => {
-        if (header === 'host') return 'any-domain.com';
+        if (header === 'host') return 'example.com';
+        if (header === 'x-forwarded-host') return 'any-domain.com';
        return null;
      });

      const originalUrl = new URL('http://localhost:3000/auth/callback');
      const result = correctOIDCUrl(mockRequest, originalUrl);

-      // Should return original URL when APP_URL is not configured
-      expect(result).toBe(originalUrl);
-      expect(result.toString()).toBe('http://localhost:3000/auth/callback');
+      // Should fall back to host header when APP_URL is not configured and forwarded host is present
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
    });

    it('should handle domains that look like subdomains but are not', () => {
      (mockRequest.headers.get as any).mockImplementation((header: string) => {
-        if (header === 'host') return 'fakeexample.com'; // Not a subdomain of example.com
+        if (header === 'host') return 'example.com';
+        if (header === 'x-forwarded-host') return 'fakeexample.com'; // Not a subdomain of example.com
        return null;
      });

      const originalUrl = new URL('http://localhost:3000/auth/callback');
      const result = correctOIDCUrl(mockRequest, originalUrl);

-      // Should prevent redirection to fake domain
-      expect(result).toBe(originalUrl);
-      expect(result.toString()).toBe('http://localhost:3000/auth/callback');
+      // Should fall back to host header
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
+    });
+
+    it('should reject invalid forwarded protocol', () => {
+      (mockRequest.headers.get as any).mockImplementation((header: string) => {
+        if (header === 'host') return 'example.com';
+        if (header === 'x-forwarded-host') return 'example.com';
+        if (header === 'x-forwarded-proto') return 'javascript'; // Invalid protocol
+        return null;
+      });
+
+      const originalUrl = new URL('http://localhost:3000/auth/callback');
+      const result = correctOIDCUrl(mockRequest, originalUrl);
+
+      // Should fall back to http protocol from URL
+      expect(result.protocol).toBe('http:');
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
+    });
+
+    it('should handle uppercase in forwarded protocol', () => {
+      (mockRequest.headers.get as any).mockImplementation((header: string) => {
+        if (header === 'host') return 'example.com';
+        if (header === 'x-forwarded-host') return 'example.com';
+        if (header === 'x-forwarded-proto') return 'HTTPS'; // Uppercase
+        return null;
+      });
+
+      const originalUrl = new URL('http://localhost:3000/auth/callback');
+      const result = correctOIDCUrl(mockRequest, originalUrl);
+
+      // Should normalize to lowercase
+      expect(result.protocol).toBe('https:');
+      expect(result.toString()).toBe('https://example.com:3000/auth/callback');
+    });
+
+    it('should handle multiple hosts in x-forwarded-host', () => {
+      (mockRequest.headers.get as any).mockImplementation((header: string) => {
+        if (header === 'host') return 'internal.com';
+        if (header === 'x-forwarded-host') return 'example.com,attacker.com'; // Multiple hosts
+        return null;
+      });
+
+      const originalUrl = new URL('http://localhost:3000/auth/callback');
+      const result = correctOIDCUrl(mockRequest, originalUrl);
+
+      // Should use the first (leftmost) host
+      expect(result.hostname).toBe('example.com');
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
+    });
+
+    it('should fall back to request host when forwarded host is invalid', () => {
+      (mockRequest.headers.get as any).mockImplementation((header: string) => {
+        if (header === 'host') return 'example.com';
+        if (header === 'x-forwarded-host') return 'evil.com'; // Invalid
+        return null;
+      });
+
+      const originalUrl = new URL('http://localhost:3000/auth/callback');
+      const result = correctOIDCUrl(mockRequest, originalUrl);
+
+      // Should fall back to request host
+      expect(result.hostname).toBe('example.com');
+      expect(result.toString()).toBe('http://example.com:3000/auth/callback');
    });
  });
 });
--- a/packages/utils/src/server/correctOIDCUrl.ts
+++ b/packages/utils/src/server/correctOIDCUrl.ts
@@ -5,29 +5,69 @@ import { validateRedirectHost } from './validateRedirectHost';

 const log = debug('lobe-oidc:correctOIDCUrl');

+// Allowed protocols for security
+const ALLOWED_PROTOCOLS = ['http', 'https'] as const;
+
 /**
 * Fix OIDC redirect URL issues in proxy environments
+ *
+ * This function:
+ * 1. Validates protocol against whitelist (http, https only)
+ * 2. Handles X-Forwarded-Host with multiple values (RFC 7239)
+ * 3. Validates X-Forwarded-Host against APP_URL to prevent open redirect attacks
+ * 4. Provides fallback logic for invalid forwarded values
+ *
+ * Note: Only X-Forwarded-Host is validated, not the Host header. This is because:
+ * - X-Forwarded-Host can be injected by attackers
+ * - Host header comes from the reverse proxy or direct access, which is trusted
+ *
 * @param req - Next.js request object
 * @param url - URL object to fix
 * @returns Fixed URL object
 */
 export const correctOIDCUrl = (req: NextRequest, url: URL): URL => {
+  log('Input URL: %s', url.toString());
+
+  // Get request headers for origin determination
  const requestHost = req.headers.get('host');
  const forwardedHost = req.headers.get('x-forwarded-host');
  const forwardedProto =
    req.headers.get('x-forwarded-proto') || req.headers.get('x-forwarded-protocol');

-  log('Input URL: %s', url.toString());
  log(
-    'Request headers - host: %s, x-forwarded-host: %s, x-forwarded-proto: %s',
+    'Getting safe origin - requestHost: %s, forwardedHost: %s, forwardedProto: %s',
    requestHost,
    forwardedHost,
    forwardedProto,
  );

-  // Determine actual hostname and protocol with fallback values
-  const actualHost = forwardedHost || requestHost;
-  const actualProto = forwardedProto || (url.protocol === 'https:' ? 'https' : 'http');
+  // Determine actual hostname with fallback values
+  // Handle multiple hosts in X-Forwarded-Host (RFC 7239: comma-separated)
+  let actualHost = forwardedHost || requestHost;
+  if (forwardedHost && forwardedHost.includes(',')) {
+    // Take the first (leftmost) host as the original client's request
+    actualHost = forwardedHost.split(',')[0]!.trim();
+    log('Multiple hosts in X-Forwarded-Host, using first: %s', actualHost);
+  }
+
+  // Determine actual protocol with validation
+  // Use URL's protocol as fallback to preserve original behavior
+  let actualProto: string | null | undefined = forwardedProto;
+  if (actualProto) {
+    // Validate protocol is http or https
+    const protoLower = actualProto.toLowerCase();
+    if (!ALLOWED_PROTOCOLS.includes(protoLower as any)) {
+      log('Warning: Invalid protocol %s, ignoring', actualProto);
+      actualProto = null;
+    } else {
+      actualProto = protoLower;
+    }
+  }
+
+  // Fallback protocol priority: URL protocol > request.nextUrl.protocol > 'https'
+  if (!actualProto) {
+    actualProto = url.protocol === 'https:' ? 'https' : 'http';
+  }

  // If unable to determine valid hostname, return original URL
  if (!actualHost || actualHost === 'null') {
@@ -35,9 +75,30 @@ export const correctOIDCUrl = (req: NextRequest, url: URL): URL => {
    return url;
  }

-  // Validate target host for security, prevent Open Redirect attacks
-  if (!validateRedirectHost(actualHost)) {
-    log('Warning: Target host %s failed validation, returning original URL', actualHost);
+  // Validate only X-Forwarded-Host for security, prevent Open Redirect attacks
+  // Host header is trusted (comes from reverse proxy or direct access)
+  if (forwardedHost && !validateRedirectHost(actualHost)) {
+    log('Warning: X-Forwarded-Host %s failed validation, falling back to request host', actualHost);
+    // Try to fall back to request host if forwarded host is invalid
+    if (requestHost) {
+      actualHost = requestHost;
+    } else {
+      // No valid host available
+      log('Error: No valid host available after validation, returning original URL');
+      return url;
+    }
+  }
+
+  // Build safe origin
+  const safeOrigin = `${actualProto}://${actualHost}`;
+  log('Safe origin: %s', safeOrigin);
+
+  // Parse safe origin to get hostname and protocol
+  let safeOriginUrl: URL;
+  try {
+    safeOriginUrl = new URL(safeOrigin);
+  } catch (error) {
+    log('Error parsing safe origin: %O', error);
    return url;
  }

@@ -46,24 +107,28 @@ export const correctOIDCUrl = (req: NextRequest, url: URL): URL => {
    url.hostname === 'localhost' ||
    url.hostname === '127.0.0.1' ||
    url.hostname === '0.0.0.0' ||
-    url.hostname !== actualHost;
+    url.hostname !== safeOriginUrl.hostname;

-  if (needsCorrection) {
-    log('URL needs correction. Original hostname: %s, correcting to: %s', url.hostname, actualHost);
-
-    try {
-      const correctedUrl = new URL(url.toString());
-      correctedUrl.protocol = actualProto + ':';
-      correctedUrl.host = actualHost;
-
-      log('Corrected URL: %s', correctedUrl.toString());
-      return correctedUrl;
-    } catch (error) {
-      log('Error creating corrected URL, returning original: %O', error);
-      return url;
-    }
+  if (!needsCorrection) {
+    log('URL does not need correction, returning original: %s', url.toString());
+    return url;
  }

-  log('URL does not need correction, returning original: %s', url.toString());
-  return url;
+  log(
+    'URL needs correction. Original hostname: %s, correcting to: %s',
+    url.hostname,
+    safeOriginUrl.hostname,
+  );
+
+  try {
+    const correctedUrl = new URL(url.toString());
+    correctedUrl.protocol = safeOriginUrl.protocol;
+    correctedUrl.host = safeOriginUrl.host;
+
+    log('Corrected URL: %s', correctedUrl.toString());
+    return correctedUrl;
+  } catch (error) {
+    log('Error creating corrected URL, returning original: %O', error);
+    return url;
+  }
 };
--- a/packages/utils/src/server/index.ts
+++ b/packages/utils/src/server/index.ts
@@ -3,4 +3,5 @@ export * from './correctOIDCUrl';
 export * from './response';
 export * from './responsive';
 export * from './sse';
+export * from './validateRedirectHost';
 export * from './xor';
--- a/src/app/(backend)/oidc/callback/desktop/route.ts
+++ b/src/app/(backend)/oidc/callback/desktop/route.ts
@@ -10,41 +10,15 @@ const log = debug('lobe-oidc:callback:desktop');
 const errorPathname = '/oauth/callback/error';

 /**
- * 安全地构建重定向URL
+ * 安全地构建重定向URL，使用经过验证的 correctOIDCUrl 防止开放重定向攻击
 */
 const buildRedirectUrl = (req: NextRequest, pathname: string): URL => {
-  const forwardedHost = req.headers.get('x-forwarded-host');
-  const requestHost = req.headers.get('host');
-  const forwardedProto =
-    req.headers.get('x-forwarded-proto') || req.headers.get('x-forwarded-protocol');
+  // 使用 req.nextUrl 作为基础URL，然后通过 correctOIDCUrl 进行验证和修正
+  const baseUrl = req.nextUrl.clone();
+  baseUrl.pathname = pathname;

-  // 确定实际的主机名，提供后备值
-  const actualHost = forwardedHost || requestHost;
-  const actualProto = forwardedProto || 'https';
-
-  log(
-    'Building redirect URL - host: %s, proto: %s, pathname: %s',
-    actualHost,
-    actualProto,
-    pathname,
-  );
-
-  // 如果主机名仍然无效，使用req.nextUrl作为后备
-  if (!actualHost) {
-    log('Warning: Invalid host detected, using req.nextUrl as fallback');
-    const fallbackUrl = req.nextUrl.clone();
-    fallbackUrl.pathname = pathname;
-    return fallbackUrl;
-  }
-
-  try {
-    return new URL(`${actualProto}://${actualHost}${pathname}`);
-  } catch (error) {
-    log('Error constructing URL, using req.nextUrl as fallback: %O', error);
-    const fallbackUrl = req.nextUrl.clone();
-    fallbackUrl.pathname = pathname;
-    return fallbackUrl;
-  }
+  // correctOIDCUrl 会验证 X-Forwarded-* 头部并防止开放重定向攻击
+  return correctOIDCUrl(req, baseUrl);
 };

 export const GET = async (req: NextRequest) => {
@@ -82,9 +56,6 @@ export const GET = async (req: NextRequest) => {
    log('Request x-forwarded-proto: %s', req.headers.get('x-forwarded-proto'));
    log('Constructed success URL: %s', successUrl.toString());

-    const correctedUrl = correctOIDCUrl(req, successUrl);
-    log('Final redirect URL: %s', correctedUrl.toString());
-
    // cleanup expired
    after(async () => {
      const cleanedCount = await authHandoffModel.cleanupExpired();
@@ -92,7 +63,7 @@ export const GET = async (req: NextRequest) => {
      log('Cleaned up %d expired handoff records', cleanedCount);
    });

-    return NextResponse.redirect(correctedUrl);
+    return NextResponse.redirect(successUrl);
  } catch (error) {
    log('Error in OIDC callback: %O', error);

--- a/src/libs/next/proxy/define-config.ts
+++ b/src/libs/next/proxy/define-config.ts
@@ -234,8 +234,10 @@ export function defineConfig() {
      // ref: https://authjs.dev/getting-started/session-management/protecting
      if (isProtected) {
        logNextAuth('Request a protected route, redirecting to sign-in page');
-        const nextLoginUrl = new URL('/next-auth/signin', req.nextUrl.origin);
-        nextLoginUrl.searchParams.set('callbackUrl', req.nextUrl.href);
+        const authUrl = authEnv.NEXT_PUBLIC_AUTH_URL;
+        const callbackUrl = `${authUrl}${req.nextUrl.pathname}${req.nextUrl.search}`;
+        const nextLoginUrl = new URL('/next-auth/signin', authUrl);
+        nextLoginUrl.searchParams.set('callbackUrl', callbackUrl);
        const hl = req.nextUrl.searchParams.get('hl');
        if (hl) {
          nextLoginUrl.searchParams.set('hl', hl);
@@ -320,8 +322,10 @@ export function defineConfig() {
      // If request a protected route, redirect to sign-in page
      if (isProtected) {
        logBetterAuth('Request a protected route, redirecting to sign-in page');
-        const signInUrl = new URL('/signin', req.nextUrl.origin);
-        signInUrl.searchParams.set('callbackUrl', req.nextUrl.href);
+        const authUrl = authEnv.NEXT_PUBLIC_AUTH_URL;
+        const callbackUrl = `${authUrl}${req.nextUrl.pathname}${req.nextUrl.search}`;
+        const signInUrl = new URL('/signin', authUrl);
+        signInUrl.searchParams.set('callbackUrl', callbackUrl);
        const hl = req.nextUrl.searchParams.get('hl');
        if (hl) {
          signInUrl.searchParams.set('hl', hl);