diff --git a/e2e/features/auth/redirect-security.feature b/e2e/features/auth/redirect-security.feature new file mode 100644 index 00000000000..7df3a1dfff6 --- /dev/null +++ b/e2e/features/auth/redirect-security.feature @@ -0,0 +1,15 @@ +@auth @redirect-security +Feature: Safe sign-in redirects + + @authenticated + Scenario: Ignore an external redirect target for an authenticated user + Given I am signed in as the default E2E admin + When I open the sign-in page with redirect target "https://google.com" + Then I should be on the console home + + @unauthenticated + Scenario: Ignore an external redirect target after signing in + Given I am not signed in + When I open the sign-in page with redirect target "https://google.com" + And I sign in as the default E2E admin + Then I should be on the console home diff --git a/e2e/features/step-definitions/auth/sign-in.steps.ts b/e2e/features/step-definitions/auth/sign-in.steps.ts index fcf106048b9..efe9357df13 100644 --- a/e2e/features/step-definitions/auth/sign-in.steps.ts +++ b/e2e/features/step-definitions/auth/sign-in.steps.ts @@ -6,6 +6,15 @@ When('I open the sign-in page', async function (this: DifyWorld) { await this.getPage().goto('/signin') }) +When( + 'I open the sign-in page with redirect target {string}', + async function (this: DifyWorld, redirectTarget: string) { + const searchParams = new URLSearchParams({ redirect_url: redirectTarget }) + + await this.getPage().goto(`/signin?${searchParams}`) + }, +) + When('I sign in as the default E2E admin', async function (this: DifyWorld) { const page = this.getPage() diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index 9713a2afc3b..9c14671da53 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -218,9 +218,6 @@ } }, "web/app/(shareLayout)/components/splash.tsx": { - "eslint-react/set-state-in-effect": { - "count": 1 - }, "jsx_a11y/click-events-have-key-events": { "count": 1 }, @@ -276,9 +273,6 @@ }, "no-restricted-imports": { "count": 1 - }, - "typescript/no-explicit-any": { - "count": 2 } }, "web/app/(shareLayout)/webapp-signin/normalForm.tsx": { diff --git a/web/__tests__/embedded-user-id-auth.test.tsx b/web/__tests__/embedded-user-id-auth.test.tsx index f2092832d9e..761c730aa5b 100644 --- a/web/__tests__/embedded-user-id-auth.test.tsx +++ b/web/__tests__/embedded-user-id-auth.test.tsx @@ -1,6 +1,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react' import * as React from 'react' import CheckCode from '@/app/(shareLayout)/webapp-signin/check-code/page' +import MailAndCodeAuth from '@/app/(shareLayout)/webapp-signin/components/mail-and-code-auth' import MailAndPasswordAuth from '@/app/(shareLayout)/webapp-signin/components/mail-and-password-auth' const replaceMock = vi.fn() @@ -98,6 +99,46 @@ describe('embedded user id propagation in authentication flows', () => { expect(replaceMock).toHaveBeenCalledWith('/chatbot/test-app') }) + it('does not call password login services when the redirect target is external', async () => { + const params = new URLSearchParams() + params.set('redirect_url', 'https://evil.example/chatbot/evil-app') + useSearchParamsMock.mockReturnValue(params) + + render() + + fireEvent.change(screen.getByLabelText('login.email'), { + target: { value: 'user@example.com' }, + }) + fireEvent.change(screen.getByLabelText(/login\.password/), { + target: { value: 'strong-password' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'login.signBtn' })) + + await waitFor(() => { + expect(replaceMock).toHaveBeenCalledWith('/') + }) + expect(webAppLoginMock).not.toHaveBeenCalled() + expect(fetchAccessTokenMock).not.toHaveBeenCalled() + }) + + it('does not request an email code when the redirect target is external', async () => { + const params = new URLSearchParams() + params.set('redirect_url', 'https://evil.example/chatbot/evil-app') + useSearchParamsMock.mockReturnValue(params) + + render() + + fireEvent.change(screen.getByLabelText('login.email'), { + target: { value: 'user@example.com' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'login.signup.verifyMail' })) + + await waitFor(() => { + expect(replaceMock).toHaveBeenCalledWith('/') + }) + expect(sendWebAppEMailLoginCodeMock).not.toHaveBeenCalled() + }) + it('passes embedded user id when verifying email code', async () => { const params = new URLSearchParams() params.set('redirect_url', encodeURIComponent('/chatbot/test-app')) @@ -128,4 +169,25 @@ describe('embedded user id propagation in authentication flows', () => { expect(setWebAppPassportMock).toHaveBeenCalledWith('test-app', 'passport-token') expect(replaceMock).toHaveBeenCalledWith('/chatbot/test-app') }) + + it('does not call code login services when the redirect target is external', async () => { + const params = new URLSearchParams() + params.set('redirect_url', 'https://evil.example/chatbot/evil-app') + params.set('email', encodeURIComponent('user@example.com')) + params.set('token', encodeURIComponent('token-abc')) + useSearchParamsMock.mockReturnValue(params) + + render() + + fireEvent.change(screen.getByPlaceholderText('login.checkCode.verificationCodePlaceholder'), { + target: { value: '123456' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'login.checkCode.verify' })) + + await waitFor(() => { + expect(replaceMock).toHaveBeenCalledWith('/') + }) + expect(webAppEmailLoginWithCodeMock).not.toHaveBeenCalled() + expect(fetchAccessTokenMock).not.toHaveBeenCalled() + }) }) diff --git a/web/__tests__/embedded-user-id-store.test.tsx b/web/__tests__/embedded-user-id-store.test.tsx index d95c52d743d..5e69feabf97 100644 --- a/web/__tests__/embedded-user-id-store.test.tsx +++ b/web/__tests__/embedded-user-id-store.test.tsx @@ -111,6 +111,38 @@ describe('WebAppStoreProvider embedded user id handling', () => { expect(useGetWebAppAccessModeByCodeMock).toHaveBeenCalledWith('redirected-app') }) + it('does not derive a share code from an external redirect target', () => { + const params = new URLSearchParams() + params.set('redirect_url', 'https://evil.example/chatbot/evil-app') + navigationMocks.usePathname.mockReturnValue('/webapp-signin') + navigationMocks.useSearchParams.mockReturnValue(params) + mockGetProcessedSystemVariablesFromUrlParams.mockResolvedValue({}) + + renderWithSystemFeatures( + + + , + ) + + expect(useGetWebAppAccessModeByCodeMock).toHaveBeenCalledWith(null) + }) + + it.each(['/webapp-signin/check-code', '/console/webapp-signin/check-code'])( + 'does not derive a share code from the sign-in route %s', + (pathname) => { + navigationMocks.usePathname.mockReturnValue(pathname) + mockGetProcessedSystemVariablesFromUrlParams.mockResolvedValue({}) + + renderWithSystemFeatures( + + + , + ) + + expect(useGetWebAppAccessModeByCodeMock).toHaveBeenCalledWith(null) + }, + ) + it('hydrates embedded user and conversation ids from system variables', async () => { mockGetProcessedSystemVariablesFromUrlParams.mockResolvedValue({ user_id: 'iframe-user-123', diff --git a/web/app/(shareLayout)/components/__tests__/splash.spec.tsx b/web/app/(shareLayout)/components/__tests__/splash.spec.tsx new file mode 100644 index 00000000000..48d87b4438b --- /dev/null +++ b/web/app/(shareLayout)/components/__tests__/splash.spec.tsx @@ -0,0 +1,124 @@ +import { render, waitFor } from '@testing-library/react' +import Splash from '../splash' + +const navigationMocks = vi.hoisted(() => ({ + replace: vi.fn(), + pathname: '/chatbot/share-app', + searchParams: new URLSearchParams(), +})) + +const webAppAuthMocks = vi.hoisted(() => ({ + setWebAppAccessToken: vi.fn(), + setWebAppPassport: vi.fn(), + webAppLoginStatus: vi.fn(), + webAppLogout: vi.fn(), +})) + +const fetchAccessTokenMock = vi.hoisted(() => vi.fn()) + +const webAppState: { + shareCode: string | null + webAppAccessMode: string + embeddedUserId: string +} = { + shareCode: 'share-app', + webAppAccessMode: 'public', + embeddedUserId: 'embedded-user', +} + +vi.mock('@/context/web-app-context', () => ({ + useWebAppStore: (selector: (state: typeof webAppState) => unknown) => selector(webAppState), +})) + +vi.mock('@/next/navigation', () => ({ + usePathname: () => navigationMocks.pathname, + useRouter: () => ({ replace: navigationMocks.replace }), + useSearchParams: () => navigationMocks.searchParams, +})) + +vi.mock('@/service/share', () => ({ + fetchAccessToken: (...args: unknown[]) => fetchAccessTokenMock(...args), +})) + +vi.mock('@/service/webapp-auth', () => webAppAuthMocks) + +describe('Splash redirect security', () => { + beforeEach(() => { + vi.clearAllMocks() + webAppState.shareCode = 'share-app' + navigationMocks.pathname = '/chatbot/share-app' + navigationMocks.searchParams = new URLSearchParams({ + redirect_url: 'https://evil.example/chatbot/evil-app', + }) + webAppAuthMocks.webAppLoginStatus.mockResolvedValue({ + userLoggedIn: false, + appLoggedIn: false, + }) + }) + + it('should use the login fallback without checking auth when the redirect target is external', async () => { + render( + +
share application
+
, + ) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + expect(webAppAuthMocks.webAppLoginStatus).not.toHaveBeenCalled() + expect(fetchAccessTokenMock).not.toHaveBeenCalled() + }) + + it('should use the login fallback without checking auth when redirect_url is empty', async () => { + navigationMocks.searchParams = new URLSearchParams('redirect_url=') + + render( + +
share application
+
, + ) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + expect(webAppAuthMocks.webAppLoginStatus).not.toHaveBeenCalled() + expect(fetchAccessTokenMock).not.toHaveBeenCalled() + }) + + it('should use the fallback without checking auth when the sign-in page has no target', async () => { + navigationMocks.searchParams = new URLSearchParams() + navigationMocks.pathname = '/webapp-signin' + webAppState.shareCode = null + + render( + +
share application
+
, + ) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + expect(webAppAuthMocks.webAppLoginStatus).not.toHaveBeenCalled() + expect(fetchAccessTokenMock).not.toHaveBeenCalled() + }) + + it('should fall back before checking auth when a nested sign-in route has stale share state', async () => { + navigationMocks.searchParams = new URLSearchParams() + navigationMocks.pathname = '/webapp-signin/check-code' + webAppState.shareCode = 'previous-share-app' + + render( + +
share application
+
, + ) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + expect(webAppAuthMocks.webAppLoginStatus).not.toHaveBeenCalled() + expect(fetchAccessTokenMock).not.toHaveBeenCalled() + }) +}) diff --git a/web/app/(shareLayout)/components/splash.tsx b/web/app/(shareLayout)/components/splash.tsx index 71125b4c086..fe3360e72e0 100644 --- a/web/app/(shareLayout)/components/splash.tsx +++ b/web/app/(shareLayout)/components/splash.tsx @@ -1,11 +1,16 @@ 'use client' -import type { FC, PropsWithChildren } from 'react' +import type { PropsWithChildren } from 'react' import { useCallback, useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' +import { + isWebAppSigninPath, + resolveWebAppLoginRedirect, +} from '@/app/(shareLayout)/webapp-signin/login-redirect' import AppUnavailable from '@/app/components/base/app-unavailable' import Loading from '@/app/components/base/loading' +import { IS_CLOUD_EDITION } from '@/config' import { useWebAppStore } from '@/context/web-app-context' -import { useRouter, useSearchParams } from '@/next/navigation' +import { usePathname, useRouter, useSearchParams } from '@/next/navigation' import { fetchAccessToken } from '@/service/share' import { setWebAppAccessToken, @@ -13,12 +18,16 @@ import { webAppLoginStatus, webAppLogout, } from '@/service/webapp-auth' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' -const Splash: FC = ({ children }) => { +function Splash({ children }: PropsWithChildren) { const { t } = useTranslation() const shareCode = useWebAppStore((s) => s.shareCode) const webAppAccessMode = useWebAppStore((s) => s.webAppAccessMode) const embeddedUserId = useWebAppStore((s) => s.embeddedUserId) + const pathname = usePathname() const searchParams = useSearchParams() const router = useRouter() const redirectUrl = searchParams.get('redirect_url') @@ -29,26 +38,43 @@ const Splash: FC = ({ children }) => { const params = new URLSearchParams(searchParams) params.delete('message') params.delete('code') + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (loginRedirect) params.set('redirect_url', loginRedirect.target.href) + else params.delete('redirect_url') return `/webapp-signin?${params.toString()}` - }, [searchParams]) + }, [redirectUrl, searchParams]) const backToHome = useCallback(async () => { - await webAppLogout(shareCode!) + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + const effectiveShareCode = loginRedirect?.appCode || shareCode + if (!effectiveShareCode || (isWebAppSigninPath(pathname) && !loginRedirect)) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + return + } + + await webAppLogout(effectiveShareCode) const url = getSigninUrl() router.replace(url) - }, [getSigninUrl, router, shareCode]) + }, [getSigninUrl, pathname, redirectUrl, router, shareCode]) const [isLoading, setIsLoading] = useState(true) useEffect(() => { - if (message) { - setIsLoading(false) + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + const isSigninRoute = isWebAppSigninPath(pathname) + if ((redirectUrl !== null && !loginRedirect) || (isSigninRoute && !loginRedirect)) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) return } + const effectiveShareCode = loginRedirect?.appCode || shareCode + if (!effectiveShareCode) return + + if (message) return + if (tokenFromUrl) setWebAppAccessToken(tokenFromUrl) const redirectOrFinish = () => { - if (redirectUrl) router.replace(decodeURIComponent(redirectUrl)) + if (loginRedirect) replaceLoginRedirect(loginRedirect.target, router.replace, basePath) else setIsLoading(false) } @@ -59,7 +85,7 @@ const Splash: FC = ({ children }) => { ;(async () => { // if access mode is public, user login is always true, but the app login(passport) may be expired const { userLoggedIn, appLoggedIn } = await webAppLoginStatus( - shareCode!, + effectiveShareCode, embeddedUserId || undefined, ) if (userLoggedIn && appLoggedIn) { @@ -71,18 +97,27 @@ const Splash: FC = ({ children }) => { } else if (userLoggedIn && !appLoggedIn) { try { const { access_token } = await fetchAccessToken({ - appCode: shareCode!, + appCode: effectiveShareCode, userId: embeddedUserId || undefined, }) - setWebAppPassport(shareCode!, access_token) + setWebAppPassport(effectiveShareCode, access_token) redirectOrFinish() } catch { - await webAppLogout(shareCode!) + await webAppLogout(effectiveShareCode) proceedToAuth() } } })() - }, [shareCode, redirectUrl, router, message, webAppAccessMode, tokenFromUrl, embeddedUserId]) + }, [ + shareCode, + redirectUrl, + pathname, + router, + message, + webAppAccessMode, + tokenFromUrl, + embeddedUserId, + ]) if (message) { return ( @@ -101,6 +136,8 @@ const Splash: FC = ({ children }) => { ) } + if (!shareCode && redirectUrl === null && !isWebAppSigninPath(pathname)) return <>{children} + if (isLoading) { return (
diff --git a/web/app/(shareLayout)/webapp-signin/__tests__/login-redirect.spec.ts b/web/app/(shareLayout)/webapp-signin/__tests__/login-redirect.spec.ts new file mode 100644 index 00000000000..5a50bea8f47 --- /dev/null +++ b/web/app/(shareLayout)/webapp-signin/__tests__/login-redirect.spec.ts @@ -0,0 +1,71 @@ +import { resolveWebAppLoginRedirect } from '../login-redirect' + +describe('resolveWebAppLoginRedirect', () => { + // Covers the canonical relative redirect shape used by share applications. + describe('internal targets', () => { + it('should return the sanitized target and app code for a legacy encoded path', () => { + const result = resolveWebAppLoginRedirect( + encodeURIComponent('/chatbot/share-app?foo=bar#answer'), + 'https://self-hosted.example.com', + ) + + expect(result).toEqual({ + appCode: 'share-app', + target: { kind: 'internal', href: '/chatbot/share-app?foo=bar#answer' }, + }) + }) + + it('should preserve a nested OAuth callback without validating it as the top-level target', () => { + const redirectUrl = '/chatbot/share-app?redirect_uri=https%3A%2F%2Fclient.example%2Fcallback' + + const result = resolveWebAppLoginRedirect(redirectUrl, 'https://self-hosted.example.com') + + expect(result?.target.href).toBe(redirectUrl) + expect(result?.appCode).toBe('share-app') + }) + }) + + // Covers absolute destinations accepted by the shared login redirect policy. + describe('absolute targets', () => { + it('should accept a same-origin self-hosted URL with a custom port', () => { + const result = resolveWebAppLoginRedirect( + 'http://self-hosted.example.com:8080/chatbot/share-app', + 'http://self-hosted.example.com:8080', + ) + + expect(result).toEqual({ + appCode: 'share-app', + target: { + kind: 'absolute', + href: 'http://self-hosted.example.com:8080/chatbot/share-app', + }, + }) + }) + + it('should accept a trusted Dify subdomain URL', () => { + const result = resolveWebAppLoginRedirect( + 'https://apps.eu.dify.ai/chatbot/share-app', + 'https://cloud.dify.ai', + ) + + expect(result?.appCode).toBe('share-app') + expect(result?.target.kind).toBe('absolute') + }) + }) + + // Invalid targets must not be allowed to influence application identity. + describe('invalid targets', () => { + it.each([ + null, + '', + '/', + '/webapp-signin', + '/webapp-signin/check-code', + '/console/webapp-signin/check-code', + 'https://evil.example/chatbot/evil-app', + '//evil.example/chatbot/evil-app', + ])('should return null for %s', (redirectUrl) => { + expect(resolveWebAppLoginRedirect(redirectUrl, 'https://self-hosted.example.com')).toBeNull() + }) + }) +}) diff --git a/web/app/(shareLayout)/webapp-signin/__tests__/page.spec.tsx b/web/app/(shareLayout)/webapp-signin/__tests__/page.spec.tsx new file mode 100644 index 00000000000..d5d897569cc --- /dev/null +++ b/web/app/(shareLayout)/webapp-signin/__tests__/page.spec.tsx @@ -0,0 +1,46 @@ +import { waitFor } from '@testing-library/react' +import { renderWithSystemFeatures } from '@/__tests__/utils/mock-system-features' +import { AccessMode } from '@/models/access-control' +import WebSSOForm from '../page' + +const navigationMocks = vi.hoisted(() => ({ + replace: vi.fn(), + searchParams: new URLSearchParams(), +})) + +const webAppState = { + shareCode: 'share-app', + webAppAccessMode: AccessMode.PUBLIC, +} + +vi.mock('@/context/web-app-context', () => ({ + useWebAppStore: (selector: (state: typeof webAppState) => unknown) => selector(webAppState), +})) + +vi.mock('@/next/navigation', () => ({ + useRouter: () => ({ replace: navigationMocks.replace }), + useSearchParams: () => navigationMocks.searchParams, +})) + +vi.mock('@/service/webapp-auth', () => ({ + webAppLogout: vi.fn(), +})) + +describe('WebSSOForm redirect security', () => { + beforeEach(() => { + vi.clearAllMocks() + navigationMocks.searchParams = new URLSearchParams({ + redirect_url: 'https://evil.example/chatbot/evil-app', + }) + }) + + it('should use the login fallback when the redirect target is external', async () => { + renderWithSystemFeatures(, { + systemFeatures: { webapp_auth: { enabled: true } }, + }) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + }) +}) diff --git a/web/app/(shareLayout)/webapp-signin/check-code/page.tsx b/web/app/(shareLayout)/webapp-signin/check-code/page.tsx index 7e0c734a21d..764838a8923 100644 --- a/web/app/(shareLayout)/webapp-signin/check-code/page.tsx +++ b/web/app/(shareLayout)/webapp-signin/check-code/page.tsx @@ -3,10 +3,12 @@ import type { FormEvent } from 'react' import { Button } from '@langgenius/dify-ui/button' import { toast } from '@langgenius/dify-ui/toast' import { RiArrowLeftLine, RiMailSendFill } from '@remixicon/react' -import { useCallback, useEffect, useRef, useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' +import { resolveWebAppLoginRedirect } from '@/app/(shareLayout)/webapp-signin/login-redirect' import Input from '@/app/components/base/input' import Countdown from '@/app/components/signin/countdown' +import { IS_CLOUD_EDITION } from '@/config' import { useLocale } from '@/context/i18n' import { useWebAppStore } from '@/context/web-app-context' import { useRouter, useSearchParams } from '@/next/navigation' @@ -14,6 +16,9 @@ import { sendWebAppEMailLoginCode, webAppEmailLoginWithCode } from '@/service/co import { fetchAccessToken } from '@/service/share' import { setWebAppAccessToken, setWebAppPassport } from '@/service/webapp-auth' import { encryptVerificationCode } from '@/utils/encryption' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' export default function CheckCode() { const { t } = useTranslation() @@ -21,25 +26,25 @@ export default function CheckCode() { const searchParams = useSearchParams() const email = decodeURIComponent(searchParams.get('email') as string) const token = decodeURIComponent(searchParams.get('token') as string) - const [code, setVerifyCode] = useState('') - const [loading, setIsLoading] = useState(false) + const [code, setCode] = useState('') + const [loading, setLoading] = useState(false) const locale = useLocale() const codeInputRef = useRef(null) const redirectUrl = searchParams.get('redirect_url') const embeddedUserId = useWebAppStore((s) => s.embeddedUserId) - const getAppCodeFromRedirectUrl = useCallback(() => { - if (!redirectUrl) return null - const url = new URL(`${window.location.origin}${decodeURIComponent(redirectUrl)}`) - const appCode = url.pathname.split('/').pop() - if (!appCode) return null - - return appCode - }, [redirectUrl]) + useEffect(() => { + if (!resolveWebAppLoginRedirect(redirectUrl, window.location.origin)) + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + }, [redirectUrl, router]) const verify = async () => { + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (!loginRedirect) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + return + } try { - const appCode = getAppCodeFromRedirectUrl() if (!code.trim()) { toast.error(t(($) => $['checkCode.emptyCode'], { ns: 'login' })) return @@ -48,11 +53,7 @@ export default function CheckCode() { toast.error(t(($) => $['checkCode.invalidCode'], { ns: 'login' })) return } - if (!redirectUrl || !appCode) { - toast.error(t(($) => $['error.redirectUrlMissing'], { ns: 'login' })) - return - } - setIsLoading(true) + setLoading(true) const ret = await webAppEmailLoginWithCode({ email, code: encryptVerificationCode(code), @@ -63,16 +64,16 @@ export default function CheckCode() { setWebAppAccessToken(ret.data.access_token) } const { access_token } = await fetchAccessToken({ - appCode: appCode!, + appCode: loginRedirect.appCode, userId: embeddedUserId || undefined, }) - setWebAppPassport(appCode!, access_token) - router.replace(decodeURIComponent(redirectUrl)) + setWebAppPassport(loginRedirect.appCode, access_token) + replaceLoginRedirect(loginRedirect.target, router.replace, basePath) } } catch (error) { console.error(error) } finally { - setIsLoading(false) + setLoading(false) } } @@ -86,11 +87,17 @@ export default function CheckCode() { }, []) const resendCode = async () => { + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (!loginRedirect) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + return + } try { const ret = await sendWebAppEMailLoginCode(email, locale) if (ret.result === 'success') { const params = new URLSearchParams(searchParams) params.set('token', encodeURIComponent(ret.data)) + params.set('redirect_url', loginRedirect.target.href) router.replace(`/webapp-signin/check-code?${params.toString()}`) } } catch (error) { @@ -125,7 +132,7 @@ export default function CheckCode() { ref={codeInputRef} id="code" value={code} - onChange={(e) => setVerifyCode(e.target.value)} + onChange={(e) => setCode(e.target.value)} maxLength={6} className="mt-1" placeholder={t(($) => $['checkCode.verificationCodePlaceholder'], { ns: 'login' }) || ''} diff --git a/web/app/(shareLayout)/webapp-signin/components/__tests__/external-member-sso-auth.spec.tsx b/web/app/(shareLayout)/webapp-signin/components/__tests__/external-member-sso-auth.spec.tsx new file mode 100644 index 00000000000..d144e9f335a --- /dev/null +++ b/web/app/(shareLayout)/webapp-signin/components/__tests__/external-member-sso-auth.spec.tsx @@ -0,0 +1,70 @@ +import { waitFor } from '@testing-library/react' +import { renderWithSystemFeatures } from '@/__tests__/utils/mock-system-features' +import { SSOProtocol } from '@/features/system-features/constants' +import ExternalMemberSSOAuth from '../external-member-sso-auth' + +const navigationMocks = vi.hoisted(() => ({ + push: vi.fn(), + replace: vi.fn(), + searchParams: new URLSearchParams(), +})) + +const serviceMocks = vi.hoisted(() => ({ + fetchWebOAuth2SSOUrl: vi.fn(), + fetchWebOIDCSSOUrl: vi.fn(), + fetchWebSAMLSSOUrl: vi.fn(), +})) + +vi.mock('@/next/navigation', () => ({ + useRouter: () => ({ + push: navigationMocks.push, + replace: navigationMocks.replace, + }), + useSearchParams: () => navigationMocks.searchParams, +})) + +vi.mock('@/service/share', () => serviceMocks) + +describe('ExternalMemberSSOAuth redirect security', () => { + beforeEach(() => { + vi.clearAllMocks() + navigationMocks.searchParams = new URLSearchParams({ + redirect_url: 'https://evil.example/chatbot/evil-app', + }) + }) + + it('should use the login fallback without calling SSO when the redirect target is external', async () => { + renderWithSystemFeatures(, { + systemFeatures: { + webapp_auth: { sso_config: { protocol: SSOProtocol.SAML } }, + }, + }) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + expect(serviceMocks.fetchWebSAMLSSOUrl).not.toHaveBeenCalled() + expect(serviceMocks.fetchWebOIDCSSOUrl).not.toHaveBeenCalled() + expect(serviceMocks.fetchWebOAuth2SSOUrl).not.toHaveBeenCalled() + }) + + it.each([ + [SSOProtocol.SAML, serviceMocks.fetchWebSAMLSSOUrl], + [SSOProtocol.OIDC, serviceMocks.fetchWebOIDCSSOUrl], + [SSOProtocol.OAuth2, serviceMocks.fetchWebOAuth2SSOUrl], + ])('should send the sanitized redirect target to %s SSO', async (protocol, serviceMock) => { + navigationMocks.searchParams = new URLSearchParams({ + redirect_url: encodeURIComponent('/chatbot/share-app?foo=bar'), + }) + serviceMock.mockResolvedValue({ url: 'https://idp.example/authorize' }) + + renderWithSystemFeatures(, { + systemFeatures: { webapp_auth: { sso_config: { protocol } } }, + }) + + await waitFor(() => { + expect(serviceMock).toHaveBeenCalledWith('share-app', '/chatbot/share-app?foo=bar') + }) + expect(navigationMocks.push).toHaveBeenCalledWith('https://idp.example/authorize') + }) +}) diff --git a/web/app/(shareLayout)/webapp-signin/components/__tests__/sso-auth.spec.tsx b/web/app/(shareLayout)/webapp-signin/components/__tests__/sso-auth.spec.tsx new file mode 100644 index 00000000000..d9f8d43376e --- /dev/null +++ b/web/app/(shareLayout)/webapp-signin/components/__tests__/sso-auth.spec.tsx @@ -0,0 +1,66 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { SSOProtocol } from '@/features/system-features/constants' +import SSOAuth from '../sso-auth' + +const navigationMocks = vi.hoisted(() => ({ + push: vi.fn(), + replace: vi.fn(), + searchParams: new URLSearchParams(), +})) + +const serviceMocks = vi.hoisted(() => ({ + fetchMembersOAuth2SSOUrl: vi.fn(), + fetchMembersOIDCSSOUrl: vi.fn(), + fetchMembersSAMLSSOUrl: vi.fn(), +})) + +vi.mock('@/next/navigation', () => ({ + useRouter: () => ({ + push: navigationMocks.push, + replace: navigationMocks.replace, + }), + useSearchParams: () => navigationMocks.searchParams, +})) + +vi.mock('@/service/share', () => serviceMocks) + +describe('SSOAuth redirect security', () => { + beforeEach(() => { + vi.clearAllMocks() + navigationMocks.searchParams = new URLSearchParams({ + redirect_url: 'https://evil.example/chatbot/evil-app', + }) + }) + + it('should use the login fallback without calling SSO when the redirect target is external', async () => { + render() + + fireEvent.click(screen.getByRole('button', { name: 'login.withSSO' })) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/') + }) + expect(serviceMocks.fetchMembersSAMLSSOUrl).not.toHaveBeenCalled() + expect(serviceMocks.fetchMembersOIDCSSOUrl).not.toHaveBeenCalled() + expect(serviceMocks.fetchMembersOAuth2SSOUrl).not.toHaveBeenCalled() + }) + + it.each([ + [SSOProtocol.SAML, serviceMocks.fetchMembersSAMLSSOUrl], + [SSOProtocol.OIDC, serviceMocks.fetchMembersOIDCSSOUrl], + [SSOProtocol.OAuth2, serviceMocks.fetchMembersOAuth2SSOUrl], + ])('should send the sanitized redirect target to %s SSO', async (protocol, serviceMock) => { + navigationMocks.searchParams = new URLSearchParams({ + redirect_url: encodeURIComponent('/chatbot/share-app?foo=bar'), + }) + serviceMock.mockResolvedValue({ url: 'https://idp.example/authorize' }) + render() + + fireEvent.click(screen.getByRole('button', { name: 'login.withSSO' })) + + await waitFor(() => { + expect(serviceMock).toHaveBeenCalledWith('share-app', '/chatbot/share-app?foo=bar') + }) + expect(navigationMocks.push).toHaveBeenCalledWith('https://idp.example/authorize') + }) +}) diff --git a/web/app/(shareLayout)/webapp-signin/components/external-member-sso-auth.tsx b/web/app/(shareLayout)/webapp-signin/components/external-member-sso-auth.tsx index 3615fb44419..6e3a8bae7f9 100644 --- a/web/app/(shareLayout)/webapp-signin/components/external-member-sso-auth.tsx +++ b/web/app/(shareLayout)/webapp-signin/components/external-member-sso-auth.tsx @@ -3,12 +3,17 @@ import { toast } from '@langgenius/dify-ui/toast' import { useSuspenseQuery } from '@tanstack/react-query' import * as React from 'react' import { useCallback, useEffect } from 'react' +import { resolveWebAppLoginRedirect } from '@/app/(shareLayout)/webapp-signin/login-redirect' import AppUnavailable from '@/app/components/base/app-unavailable' import Loading from '@/app/components/base/loading' +import { IS_CLOUD_EDITION } from '@/config' import { systemFeaturesQueryOptions } from '@/features/system-features/client' import { SSOProtocol } from '@/features/system-features/constants' import { useRouter, useSearchParams } from '@/next/navigation' import { fetchWebOAuth2SSOUrl, fetchWebOIDCSSOUrl, fetchWebSAMLSSOUrl } from '@/service/share' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' const ExternalMemberSSOAuth = () => { const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) @@ -21,35 +26,29 @@ const ExternalMemberSSOAuth = () => { toast.error(message) } - const getAppCodeFromRedirectUrl = useCallback(() => { - if (!redirectUrl) return null - const url = new URL(`${window.location.origin}${decodeURIComponent(redirectUrl)}`) - const appCode = url.pathname.split('/').pop() - if (!appCode) return null - - return appCode - }, [redirectUrl]) - const handleSSOLogin = useCallback(async () => { - const appCode = getAppCodeFromRedirectUrl() - if (!appCode || !redirectUrl) { - showErrorToast('redirect url or app code is invalid.') + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (!loginRedirect) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) return } switch (systemFeatures.webapp_auth.sso_config.protocol) { case SSOProtocol.SAML: { - const samlRes = await fetchWebSAMLSSOUrl(appCode, redirectUrl) + const samlRes = await fetchWebSAMLSSOUrl(loginRedirect.appCode, loginRedirect.target.href) router.push(samlRes.url) break } case SSOProtocol.OIDC: { - const oidcRes = await fetchWebOIDCSSOUrl(appCode, redirectUrl) + const oidcRes = await fetchWebOIDCSSOUrl(loginRedirect.appCode, loginRedirect.target.href) router.push(oidcRes.url) break } case SSOProtocol.OAuth2: { - const oauth2Res = await fetchWebOAuth2SSOUrl(appCode, redirectUrl) + const oauth2Res = await fetchWebOAuth2SSOUrl( + loginRedirect.appCode, + loginRedirect.target.href, + ) router.push(oauth2Res.url) break } @@ -58,12 +57,7 @@ const ExternalMemberSSOAuth = () => { default: showErrorToast('SSO protocol is not supported.') } - }, [ - getAppCodeFromRedirectUrl, - redirectUrl, - router, - systemFeatures.webapp_auth.sso_config.protocol, - ]) + }, [redirectUrl, router, systemFeatures.webapp_auth.sso_config.protocol]) useEffect(() => { handleSSOLogin() diff --git a/web/app/(shareLayout)/webapp-signin/components/mail-and-code-auth.tsx b/web/app/(shareLayout)/webapp-signin/components/mail-and-code-auth.tsx index 5b10b54e79e..109c2f31463 100644 --- a/web/app/(shareLayout)/webapp-signin/components/mail-and-code-auth.tsx +++ b/web/app/(shareLayout)/webapp-signin/components/mail-and-code-auth.tsx @@ -1,14 +1,18 @@ import { Button } from '@langgenius/dify-ui/button' import { toast } from '@langgenius/dify-ui/toast' import { noop } from 'es-toolkit/function' -import { useState } from 'react' +import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' +import { resolveWebAppLoginRedirect } from '@/app/(shareLayout)/webapp-signin/login-redirect' import Input from '@/app/components/base/input' import { COUNT_DOWN_TIME_MS, useSetCountdownLeftTime } from '@/app/components/signin/storage' -import { emailRegex } from '@/config' +import { emailRegex, IS_CLOUD_EDITION } from '@/config' import { useLocale } from '@/context/i18n' import { useRouter, useSearchParams } from '@/next/navigation' import { sendWebAppEMailLoginCode } from '@/service/common' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' export default function MailAndCodeAuth() { const { t } = useTranslation() @@ -16,11 +20,22 @@ export default function MailAndCodeAuth() { const searchParams = useSearchParams() const emailFromLink = decodeURIComponent(searchParams.get('email') || '') const [email, setEmail] = useState(emailFromLink) - const [loading, setIsLoading] = useState(false) + const [loading, setLoading] = useState(false) const locale = useLocale() const setCountdownLeftTime = useSetCountdownLeftTime() + const redirectUrl = searchParams.get('redirect_url') + + useEffect(() => { + if (!resolveWebAppLoginRedirect(redirectUrl, window.location.origin)) + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + }, [redirectUrl, router]) const handleGetEMailVerificationCode = async () => { + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (!loginRedirect) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + return + } try { if (!email) { toast.error(t(($) => $['error.emailEmpty'], { ns: 'login' })) @@ -31,19 +46,20 @@ export default function MailAndCodeAuth() { toast.error(t(($) => $['error.emailInValid'], { ns: 'login' })) return } - setIsLoading(true) + setLoading(true) const ret = await sendWebAppEMailLoginCode(email, locale) if (ret.result === 'success') { setCountdownLeftTime(`${COUNT_DOWN_TIME_MS}`) const params = new URLSearchParams(searchParams) params.set('email', encodeURIComponent(email)) params.set('token', encodeURIComponent(ret.data)) + params.set('redirect_url', loginRedirect.target.href) router.push(`/webapp-signin/check-code?${params.toString()}`) } } catch (error) { console.error(error) } finally { - setIsLoading(false) + setLoading(false) } } diff --git a/web/app/(shareLayout)/webapp-signin/components/mail-and-password-auth.tsx b/web/app/(shareLayout)/webapp-signin/components/mail-and-password-auth.tsx index c598e2a1bda..879314c7f91 100644 --- a/web/app/(shareLayout)/webapp-signin/components/mail-and-password-auth.tsx +++ b/web/app/(shareLayout)/webapp-signin/components/mail-and-password-auth.tsx @@ -2,10 +2,11 @@ import { Button } from '@langgenius/dify-ui/button' import { toast } from '@langgenius/dify-ui/toast' import { noop } from 'es-toolkit/function' -import { useCallback, useState } from 'react' +import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' +import { resolveWebAppLoginRedirect } from '@/app/(shareLayout)/webapp-signin/login-redirect' import Input from '@/app/components/base/input' -import { emailRegex } from '@/config' +import { emailRegex, IS_CLOUD_EDITION } from '@/config' import { useLocale } from '@/context/i18n' import { useWebAppStore } from '@/context/web-app-context' import Link from '@/next/link' @@ -14,6 +15,9 @@ import { webAppLogin } from '@/service/common' import { fetchAccessToken } from '@/service/share' import { setWebAppAccessToken, setWebAppPassport } from '@/service/webapp-auth' import { encryptPassword } from '@/utils/encryption' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' type MailAndPasswordAuthProps = { isEmailSetup: boolean @@ -33,16 +37,17 @@ export default function MailAndPasswordAuth({ isEmailSetup }: MailAndPasswordAut const redirectUrl = searchParams.get('redirect_url') const embeddedUserId = useWebAppStore((s) => s.embeddedUserId) - const getAppCodeFromRedirectUrl = useCallback(() => { - if (!redirectUrl) return null - const url = new URL(`${window.location.origin}${decodeURIComponent(redirectUrl)}`) - const appCode = url.pathname.split('/').pop() - if (!appCode) return null + useEffect(() => { + if (!resolveWebAppLoginRedirect(redirectUrl, window.location.origin)) + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + }, [redirectUrl, router]) - return appCode - }, [redirectUrl]) - const appCode = getAppCodeFromRedirectUrl() const handleEmailPasswordLogin = async () => { + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (!loginRedirect) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + return + } if (!email) { toast.error(t(($) => $['error.emailEmpty'], { ns: 'login' })) return @@ -56,13 +61,9 @@ export default function MailAndPasswordAuth({ isEmailSetup }: MailAndPasswordAut return } - if (!redirectUrl || !appCode) { - toast.error(t(($) => $['error.redirectUrlMissing'], { ns: 'login' })) - return - } try { setIsLoading(true) - const loginData: Record = { + const loginData = { email, password: encryptPassword(password), language: locale, @@ -79,16 +80,21 @@ export default function MailAndPasswordAuth({ isEmailSetup }: MailAndPasswordAut } const { access_token } = await fetchAccessToken({ - appCode: appCode!, + appCode: loginRedirect.appCode, userId: embeddedUserId || undefined, }) - setWebAppPassport(appCode!, access_token) - router.replace(decodeURIComponent(redirectUrl)) + setWebAppPassport(loginRedirect.appCode, access_token) + replaceLoginRedirect(loginRedirect.target, router.replace, basePath) } else { toast.error(res.data) } - } catch (e: any) { - if (e.code === 'authentication_failed') toast.error(e.message) + } catch (error: unknown) { + const authenticationError = error as { code?: unknown; message?: unknown } + if ( + authenticationError.code === 'authentication_failed' && + typeof authenticationError.message === 'string' + ) + toast.error(authenticationError.message) } finally { setIsLoading(false) } diff --git a/web/app/(shareLayout)/webapp-signin/components/sso-auth.tsx b/web/app/(shareLayout)/webapp-signin/components/sso-auth.tsx index 4a4ef60d9c5..6792ab98d91 100644 --- a/web/app/(shareLayout)/webapp-signin/components/sso-auth.tsx +++ b/web/app/(shareLayout)/webapp-signin/components/sso-auth.tsx @@ -1,10 +1,11 @@ 'use client' -import type { FC } from 'react' import { Button } from '@langgenius/dify-ui/button' import { toast } from '@langgenius/dify-ui/toast' -import { useCallback, useState } from 'react' +import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' +import { resolveWebAppLoginRedirect } from '@/app/(shareLayout)/webapp-signin/login-redirect' import { Lock01 } from '@/app/components/base/icons/src/vender/solid/security' +import { IS_CLOUD_EDITION } from '@/config' import { SSOProtocol } from '@/features/system-features/constants' import { useRouter, useSearchParams } from '@/next/navigation' import { @@ -12,37 +13,37 @@ import { fetchMembersOIDCSSOUrl, fetchMembersSAMLSSOUrl, } from '@/service/share' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' type SSOAuthProps = { protocol: string } -const SSOAuth: FC = ({ protocol }) => { +function SSOAuth({ protocol }: SSOAuthProps) { const router = useRouter() const { t } = useTranslation() const searchParams = useSearchParams() const redirectUrl = searchParams.get('redirect_url') - const getAppCodeFromRedirectUrl = useCallback(() => { - if (!redirectUrl) return null - const url = new URL(`${window.location.origin}${decodeURIComponent(redirectUrl)}`) - const appCode = url.pathname.split('/').pop() - if (!appCode) return null - - return appCode - }, [redirectUrl]) const [isLoading, setIsLoading] = useState(false) + useEffect(() => { + if (!resolveWebAppLoginRedirect(redirectUrl, window.location.origin)) + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + }, [redirectUrl, router]) + const handleSSOLogin = () => { - const appCode = getAppCodeFromRedirectUrl() - if (!redirectUrl || !appCode) { - toast.error(t(($) => $['error.invalidRedirectUrlOrAppCode'], { ns: 'login' })) + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (!loginRedirect) { + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) return } setIsLoading(true) if (protocol === SSOProtocol.SAML) { - fetchMembersSAMLSSOUrl(appCode, redirectUrl) + fetchMembersSAMLSSOUrl(loginRedirect.appCode, loginRedirect.target.href) .then((res) => { router.push(res.url) }) @@ -50,7 +51,7 @@ const SSOAuth: FC = ({ protocol }) => { setIsLoading(false) }) } else if (protocol === SSOProtocol.OIDC) { - fetchMembersOIDCSSOUrl(appCode, redirectUrl) + fetchMembersOIDCSSOUrl(loginRedirect.appCode, loginRedirect.target.href) .then((res) => { router.push(res.url) }) @@ -58,7 +59,7 @@ const SSOAuth: FC = ({ protocol }) => { setIsLoading(false) }) } else if (protocol === SSOProtocol.OAuth2) { - fetchMembersOAuth2SSOUrl(appCode, redirectUrl) + fetchMembersOAuth2SSOUrl(loginRedirect.appCode, loginRedirect.target.href) .then((res) => { router.push(res.url) }) diff --git a/web/app/(shareLayout)/webapp-signin/login-redirect.ts b/web/app/(shareLayout)/webapp-signin/login-redirect.ts new file mode 100644 index 00000000000..54d39df8b0c --- /dev/null +++ b/web/app/(shareLayout)/webapp-signin/login-redirect.ts @@ -0,0 +1,50 @@ +import type { LoginRedirectTarget } from '@/utils/login-redirect' +import { resolveLoginRedirectTarget } from '@/utils/login-redirect' + +const INTERNAL_PATH_PARSE_BASE = 'https://login-redirect.invalid' + +export type WebAppLoginRedirect = { + appCode: string + target: LoginRedirectTarget +} + +export function isWebAppSigninPath(pathname: string): boolean { + let candidate = pathname + + for (let decodeCount = 0; decodeCount <= 2; decodeCount += 1) { + if (candidate.split('/').some((segment) => segment === 'webapp-signin')) return true + + try { + const decoded = decodeURIComponent(candidate) + if (decoded === candidate) return false + candidate = decoded + } catch { + return false + } + } + + return false +} + +export function resolveWebAppLoginRedirect( + raw: string | null, + currentOrigin?: string, +): WebAppLoginRedirect | null { + const target = resolveLoginRedirectTarget(raw, { + allowSameOriginAbsolute: Boolean(currentOrigin), + currentOrigin, + }) + if (!target) return null + + try { + const url = new URL(target.href, currentOrigin || INTERNAL_PATH_PARSE_BASE) + if (isWebAppSigninPath(url.pathname)) return null + + const appCode = url.pathname.split('/').filter(Boolean).at(-1) + if (!appCode) return null + + return { appCode, target } + } catch { + return null + } +} diff --git a/web/app/(shareLayout)/webapp-signin/page.tsx b/web/app/(shareLayout)/webapp-signin/page.tsx index 862f44966e8..6c6eb382b05 100644 --- a/web/app/(shareLayout)/webapp-signin/page.tsx +++ b/web/app/(shareLayout)/webapp-signin/page.tsx @@ -1,19 +1,28 @@ 'use client' -import type { FC } from 'react' import { useSuspenseQuery } from '@tanstack/react-query' import * as React from 'react' -import { useCallback } from 'react' +import { useCallback, useEffect, useSyncExternalStore } from 'react' import { useTranslation } from 'react-i18next' import AppUnavailable from '@/app/components/base/app-unavailable' +import Loading from '@/app/components/base/loading' +import { IS_CLOUD_EDITION } from '@/config' import { useWebAppStore } from '@/context/web-app-context' import { systemFeaturesQueryOptions } from '@/features/system-features/client' import { AccessMode } from '@/models/access-control' import { useRouter, useSearchParams } from '@/next/navigation' import { webAppLogout } from '@/service/webapp-auth' +import { getClientLoginFallback } from '@/utils/login-redirect' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' import ExternalMemberSsoAuth from './components/external-member-sso-auth' +import { resolveWebAppLoginRedirect } from './login-redirect' import NormalForm from './normalForm' -const WebSSOForm: FC = () => { +const subscribeToOrigin = () => () => {} +const getClientOrigin = () => window.location.origin +const getServerOrigin = () => undefined + +function WebSSOForm() { const { t } = useTranslation() const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) const webAppAccessMode = useWebAppStore((s) => s.webAppAccessMode) @@ -21,10 +30,18 @@ const WebSSOForm: FC = () => { const router = useRouter() const redirectUrl = searchParams.get('redirect_url') + const currentOrigin = useSyncExternalStore(subscribeToOrigin, getClientOrigin, getServerOrigin) + const loginRedirect = resolveWebAppLoginRedirect(redirectUrl, currentOrigin) + + useEffect(() => { + if (!resolveWebAppLoginRedirect(redirectUrl, window.location.origin)) + replaceLoginRedirect(getClientLoginFallback(IS_CLOUD_EDITION), router.replace, basePath) + }, [redirectUrl, router]) const getSigninUrl = useCallback(() => { const params = new URLSearchParams() - params.append('redirect_url', redirectUrl || '') + const resolvedRedirect = resolveWebAppLoginRedirect(redirectUrl, window.location.origin) + if (resolvedRedirect) params.set('redirect_url', resolvedRedirect.target.href) return `/webapp-signin?${params.toString()}` }, [redirectUrl]) @@ -35,13 +52,10 @@ const WebSSOForm: FC = () => { router.replace(url) }, [getSigninUrl, router, shareCode]) - if (!redirectUrl) { + if (!loginRedirect) { return (
- $['common.appUnavailable'], { ns: 'share' })} - unknownReason="redirect url is invalid." - /> +
) } diff --git a/web/app/auth/refresh/__tests__/route.spec.ts b/web/app/auth/refresh/__tests__/route.spec.ts index feec5c0be80..68622e05320 100644 --- a/web/app/auth/refresh/__tests__/route.spec.ts +++ b/web/app/auth/refresh/__tests__/route.spec.ts @@ -4,12 +4,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ basePath: '', + isCloudEdition: false, })) vi.mock('@/config', () => ({ API_PREFIX: 'http://localhost:5001/console/api', CSRF_COOKIE_NAME: () => 'csrf_token', CSRF_HEADER_NAME: 'X-CSRF-Token', + get IS_CLOUD_EDITION() { + return mocks.isCloudEdition + }, })) vi.mock('server-only', () => ({})) @@ -48,6 +52,7 @@ describe('auth refresh route', () => { vi.resetModules() vi.unstubAllGlobals() mocks.basePath = '' + mocks.isCloudEdition = false }) it('should refresh cookies and redirect back to the requested path', async () => { @@ -121,6 +126,112 @@ describe('auth refresh route', () => { expect(response.headers.get('location')).toBe('/signin?redirect_url=%2F') }) + it.each([ + ['a protocol-relative URL', '%2F%2Fevil.example'], + ['a backslash URL', '%2F%5Cevil.example'], + ['an encoded protocol-relative URL', '%252F%252Fevil.example'], + ['an HTTP Dify URL', 'http%3A%2F%2Fdocs.dify.ai%2Fapps'], + ['a Dify URL with a non-standard port', 'https%3A%2F%2Fdocs.dify.ai%3A444%2Fapps'], + ['a Dify lookalike URL', 'https%3A%2F%2Fdify.ai.evil.example%2Fapps'], + ['a URL with userinfo', 'https%3A%2F%2Fuser%3Apass%40dify.ai%2Fapps'], + ])('should use the self-hosted fallback for %s', async (_, redirectUrl) => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + `http://localhost:3000/auth/refresh?redirect_url=${redirectUrl}`, + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/signin?redirect_url=%2F') + }) + + it('should reject an absolute redirect that matches an internal proxy origin', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'http://internal-service:3000/auth/refresh?redirect_url=http%3A%2F%2Finternal-service%3A3000%2Fapps', + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/signin?redirect_url=%2F') + }) + + it('should reject a same-origin absolute redirect whose path starts with two slashes', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'https://cloud.dify.ai/auth/refresh?redirect_url=https%3A%2F%2Fcloud.dify.ai%2F%2Fevil.example', + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/signin?redirect_url=%2F') + }) + + it('should accept a trusted Dify HTTPS redirect and preserve its query and fragment', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'http://localhost:3000/auth/refresh?redirect_url=https%3A%2F%2Fdocs.eu.dify.ai%2Fapps%3Fcategory%3Dworkflow%23recent', + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe( + 'https://docs.eu.dify.ai/apps?category=workflow#recent', + ) + }) + + it('should accept a once-encoded legacy internal redirect', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'http://localhost:3000/auth/refresh?redirect_url=%252Fapps%253Fcategory%253Dworkflow', + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/apps?category=workflow') + }) + + it('should preserve a nested OAuth redirect URI without validating it as the top-level target', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + const redirectUrl = new URLSearchParams({ + redirect_url: + '/account/oauth/authorize?client_id=client&redirect_uri=https%3A%2F%2Fclient.example%2Fcallback', + }) + + const response = await GET( + createRequest( + `http://localhost:3000/auth/refresh?${redirectUrl}`, + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe( + '/account/oauth/authorize?client_id=client&redirect_uri=https%3A%2F%2Fclient.example%2Fcallback', + ) + }) + it('should default missing redirect targets to the home path', async () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) const { GET } = await import('../route') @@ -133,6 +244,75 @@ describe('auth refresh route', () => { expect(response.headers.get('location')).toBe('/signin?redirect_url=%2F') }) + it('should use the Cloud home as the fallback after a successful refresh', async () => { + mocks.isCloudEdition = true + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'https://cloud.dify.ai/auth/refresh?redirect_url=https%3A%2F%2Fevil.example', + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('https://cloud.dify.ai/') + }) + + it('should carry the Cloud fallback through signin when refresh fails', async () => { + mocks.isCloudEdition = true + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'https://cloud.dify.ai/auth/refresh?redirect_url=https%3A%2F%2Fevil.example', + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe( + '/signin?redirect_url=https%3A%2F%2Fcloud.dify.ai%2F', + ) + }) + + it('should use the Cloud home when a trusted absolute target loops back to auth refresh', async () => { + mocks.isCloudEdition = true + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'https://cloud.dify.ai/auth/refresh?redirect_url=https%3A%2F%2Fcloud.dify.ai%2Fauth%2Frefresh', + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('https://cloud.dify.ai/') + }) + + it.each(['/auth/refresh/', '/auth/%72efresh'])( + 'should fall back after refresh succeeds when %s resolves to auth refresh', + async (redirectUrl) => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + const searchParams = new URLSearchParams({ redirect_url: redirectUrl }) + + const response = await GET( + createRequest( + `http://localhost:3000/auth/refresh?${searchParams}`, + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/') + }, + ) + it('should not leak internal request origin when redirecting to signin', async () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) const { GET } = await import('../route') @@ -179,6 +359,38 @@ describe('auth refresh route', () => { ]) }) + it('should add the base path to an unprefixed internal target after refresh succeeds', async () => { + mocks.basePath = '/console' + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'http://localhost:3000/console/auth/refresh?redirect_url=%2Fapps', + 'refresh_token=old-refresh', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/console/apps') + }) + + it('should add the base path to the signin redirect target after refresh fails', async () => { + mocks.basePath = '/console' + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'http://localhost:3000/console/auth/refresh?redirect_url=%2Fapps', + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/console/signin?redirect_url=%2Fconsole%2Fapps') + }) + it('should fall back to the base path home when base path refresh redirects to itself', async () => { mocks.basePath = '/console' vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) @@ -194,4 +406,37 @@ describe('auth refresh route', () => { expect(response.status).toBe(303) expect(response.headers.get('location')).toBe('/console/signin?redirect_url=%2Fconsole%2F') }) + + it('should fall back when an unprefixed refresh target resolves to the base path route', async () => { + mocks.basePath = '/console' + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + + const response = await GET( + createRequest( + 'http://localhost:3000/console/auth/refresh?redirect_url=%2Fauth%2Frefresh', + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/console/signin?redirect_url=%2Fconsole%2F') + }) + + it('should fall back when repeated slashes resolve to the base path refresh route', async () => { + mocks.basePath = '/console' + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 401 }))) + const { GET } = await import('../route') + const searchParams = new URLSearchParams({ redirect_url: '/auth//refresh' }) + + const response = await GET( + createRequest( + `http://localhost:3000/console/auth/refresh?${searchParams}`, + 'refresh_token=expired', + ), + ) + + expect(response.status).toBe(303) + expect(response.headers.get('location')).toBe('/console/signin?redirect_url=%2Fconsole%2F') + }) }) diff --git a/web/app/auth/refresh/route.ts b/web/app/auth/refresh/route.ts index 508fa2bb622..b231840a244 100644 --- a/web/app/auth/refresh/route.ts +++ b/web/app/auth/refresh/route.ts @@ -1,25 +1,58 @@ +import type { LoginRedirectTarget } from '@/utils/login-redirect' +import { IS_CLOUD_EDITION } from '@/config' import { resolveServerConsoleApiUrl } from '@/service/server' +import { getServerLoginFallback, resolveLoginRedirectTarget } from '@/utils/login-redirect' import { basePath } from '@/utils/var' const REFRESH_TOKEN_PATH = '/refresh-token' const AUTH_REFRESH_PATH = `${basePath}/auth/refresh` -const DEFAULT_REDIRECT_PATH = `${basePath}/` +const INTERNAL_PATH_PARSE_BASE = 'https://login-redirect.invalid' -const resolveSafeRedirectPath = (request: Request) => { +function normalizeRoutePathname(pathname: string) { + let decodedPathname = pathname + for (let decodeCount = 0; decodeCount < 2; decodeCount += 1) { + try { + const nextPathname = decodeURIComponent(decodedPathname) + if (nextPathname === decodedPathname) break + decodedPathname = nextPathname + } catch { + break + } + } + + const collapsedPathname = decodedPathname.replace(/\/{2,}/g, '/') + if (collapsedPathname === '/') return collapsedPathname + return collapsedPathname.replace(/\/+$/, '') +} + +const addBasePathToInternalTarget = (target: LoginRedirectTarget): LoginRedirectTarget => { + if (!basePath || target.kind !== 'internal') return target + + const targetUrl = new URL(target.href, INTERNAL_PATH_PARSE_BASE) + if (targetUrl.pathname === basePath || targetUrl.pathname.startsWith(`${basePath}/`)) + return target + + return { kind: 'internal', href: `${basePath}${target.href}` } +} + +const resolveSafeRedirectTarget = (request: Request): LoginRedirectTarget => { const requestUrl = new URL(request.url) const redirectUrl = requestUrl.searchParams.get('redirect_url') + const fallback = getServerLoginFallback(IS_CLOUD_EDITION, basePath) - if (!redirectUrl) return DEFAULT_REDIRECT_PATH + if (!redirectUrl) return fallback - try { - const target = new URL(redirectUrl, requestUrl.origin) - if (target.origin !== requestUrl.origin) return DEFAULT_REDIRECT_PATH - if (target.pathname === AUTH_REFRESH_PATH) return DEFAULT_REDIRECT_PATH + const target = resolveLoginRedirectTarget(redirectUrl, { + allowSameOriginAbsolute: false, + }) + if (!target) return fallback - return `${target.pathname}${target.search}` - } catch { - return DEFAULT_REDIRECT_PATH - } + const normalizedTarget = addBasePathToInternalTarget(target) + const targetUrl = new URL(normalizedTarget.href, INTERNAL_PATH_PARSE_BASE) + if (normalizeRoutePathname(targetUrl.pathname) === normalizeRoutePathname(AUTH_REFRESH_PATH)) + return fallback + + return normalizedTarget } const getSetCookieHeaders = (headers: Headers) => { @@ -50,15 +83,17 @@ const createRedirectResponse = (pathname: string, setCookies: string[] = []) => }) } -const createSigninRedirectResponse = (redirectPath: string) => - createRedirectResponse(`${basePath}/signin?redirect_url=${encodeURIComponent(redirectPath)}`) +const createSigninRedirectResponse = (redirectTarget: LoginRedirectTarget) => + createRedirectResponse( + `${basePath}/signin?redirect_url=${encodeURIComponent(redirectTarget.href)}`, + ) export async function GET(request: Request) { - const redirectPath = resolveSafeRedirectPath(request) + const redirectTarget = resolveSafeRedirectTarget(request) const refreshUrl = resolveServerConsoleApiUrl(REFRESH_TOKEN_PATH) const cookie = request.headers.get('cookie') - if (!refreshUrl || !cookie) return createSigninRedirectResponse(redirectPath) + if (!refreshUrl || !cookie) return createSigninRedirectResponse(redirectTarget) try { const response = await fetch(refreshUrl, { @@ -70,10 +105,10 @@ export async function GET(request: Request) { cache: 'no-store', }) - if (!response.ok) return createSigninRedirectResponse(redirectPath) + if (!response.ok) return createSigninRedirectResponse(redirectTarget) - return createRedirectResponse(redirectPath, getSetCookieHeaders(response.headers)) + return createRedirectResponse(redirectTarget.href, getSetCookieHeaders(response.headers)) } catch { - return createSigninRedirectResponse(redirectPath) + return createSigninRedirectResponse(redirectTarget) } } diff --git a/web/app/signin/__tests__/normal-form.spec.tsx b/web/app/signin/__tests__/normal-form.spec.tsx index f5dfd5076d4..14131c03942 100644 --- a/web/app/signin/__tests__/normal-form.spec.tsx +++ b/web/app/signin/__tests__/normal-form.spec.tsx @@ -114,6 +114,21 @@ describe('NormalForm', () => { expect(mockReplace).toHaveBeenCalledWith('/') }) }) + + it('should send logged-in visitors with an external redirect target to the console home', async () => { + mockUseSearchParams.mockReturnValue( + new URLSearchParams('redirect_url=https%3A%2F%2Fgoogle.com'), + ) + mockUseQuery + .mockReturnValueOnce(loggedInQueryResult as unknown as ReturnType) + .mockReturnValueOnce(nonInviteQueryResult as unknown as ReturnType) + + render() + + await waitFor(() => { + expect(mockReplace).toHaveBeenCalledWith('/') + }) + }) }) describe('Invite Redirects', () => { diff --git a/web/app/signin/check-code/__tests__/page.spec.tsx b/web/app/signin/check-code/__tests__/page.spec.tsx new file mode 100644 index 00000000000..1ae3f8a63b3 --- /dev/null +++ b/web/app/signin/check-code/__tests__/page.spec.tsx @@ -0,0 +1,133 @@ +import type { GetAccountProfileResponse } from '@dify/contracts/api/console/account/types.gen' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { userProfileQueryOptions } from '@/features/account-profile/client' +import { emailLoginWithCode } from '@/service/common' +import CheckCode from '../page' + +const navigationMocks = vi.hoisted(() => ({ + back: vi.fn(), + replace: vi.fn(), + searchParams: new URLSearchParams(), +})) + +const serviceBaseMocks = vi.hoisted(() => ({ + get: vi.fn(), +})) + +vi.mock('@/app/components/base/amplitude', () => ({ + trackEvent: vi.fn(), +})) + +vi.mock('@/next/navigation', () => ({ + useRouter: () => ({ + back: navigationMocks.back, + replace: navigationMocks.replace, + }), + useSearchParams: () => navigationMocks.searchParams, +})) + +vi.mock('@/service/base', () => serviceBaseMocks) + +vi.mock('@/service/common', () => ({ + emailLoginWithCode: vi.fn(), + sendEMailLoginCode: vi.fn(), +})) + +vi.mock('@/utils/encryption', () => ({ + encryptVerificationCode: (code: string) => code, +})) + +vi.mock('@/utils/timezone', () => ({ + getBrowserTimezone: () => 'Asia/Singapore', +})) + +function createQueryClient() { + return new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }) +} + +const accountProfile: GetAccountProfileResponse = { + avatar: null, + avatar_url: null, + created_at: 1_700_000_000, + email: 'user@example.com', + id: 'account-id', + interface_language: 'en-US', + interface_theme: 'light', + is_password_set: true, + last_login_at: 1_700_000_000, + last_login_ip: '127.0.0.1', + name: 'User', + timezone: 'Asia/Singapore', +} + +describe('CheckCode', () => { + beforeEach(() => { + vi.clearAllMocks() + navigationMocks.searchParams = new URLSearchParams({ + email: 'user@example.com', + redirect_url: '/apps', + token: 'email-login-token', + }) + vi.mocked(emailLoginWithCode).mockResolvedValue({ result: 'success' }) + }) + + describe('Post-login profile bootstrap', () => { + it('should resolve an inactive profile query before navigating to the console home', async () => { + const user = userEvent.setup() + const queryClient = createQueryClient() + const profileQueryOptions = userProfileQueryOptions() + const profileQueryKey = profileQueryOptions.queryKey + let resolveProfileResponse: (response: Response) => void = () => {} + const profileResponse = new Promise((resolve) => { + resolveProfileResponse = resolve + }) + serviceBaseMocks.get + .mockRejectedValueOnce(new Response(null, { status: 401 })) + .mockReturnValueOnce(profileResponse) + await queryClient.prefetchQuery(profileQueryOptions) + expect(queryClient.getQueryState(profileQueryKey)?.status).toBe('error') + + render( + + + , + ) + + await user.type(screen.getByLabelText('login.checkCode.verificationCode'), '123456') + await user.click(screen.getByRole('button', { name: 'login.checkCode.verify' })) + + await waitFor(() => { + expect(serviceBaseMocks.get).toHaveBeenCalledTimes(2) + }) + expect(queryClient.getQueryState(profileQueryKey)).toMatchObject({ + fetchStatus: 'fetching', + status: 'pending', + }) + expect(navigationMocks.replace).not.toHaveBeenCalled() + + resolveProfileResponse( + new Response(JSON.stringify(accountProfile), { + headers: { + 'content-type': 'application/json', + 'x-env': 'DEVELOPMENT', + 'x-version': '1.0.0', + }, + status: 200, + }), + ) + + await waitFor(() => { + expect(navigationMocks.replace).toHaveBeenCalledWith('/apps') + }) + expect(queryClient.getQueryState(profileQueryKey)?.status).toBe('success') + }) + }) +}) diff --git a/web/app/signin/check-code/page.tsx b/web/app/signin/check-code/page.tsx index 14d4914ab23..422bdb175a8 100644 --- a/web/app/signin/check-code/page.tsx +++ b/web/app/signin/check-code/page.tsx @@ -10,11 +10,13 @@ import { trackEvent } from '@/app/components/base/amplitude' import Input from '@/app/components/base/input' import Countdown from '@/app/components/signin/countdown' import { useLocale } from '@/context/i18n' +import { userProfileQueryOptions } from '@/features/account-profile/client' import { useRouter, useSearchParams } from '@/next/navigation' -import { consoleQuery } from '@/service/client' import { emailLoginWithCode, sendEMailLoginCode } from '@/service/common' import { encryptVerificationCode } from '@/utils/encryption' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' import { getBrowserTimezone } from '@/utils/timezone' +import { basePath } from '@/utils/var' import { resolvePostLoginRedirect } from '../utils/post-login-redirect' export default function CheckCode() { @@ -59,9 +61,10 @@ export default function CheckCode() { if (invite_token) { router.replace(`/signin/invite-settings?${searchParams.toString()}`) } else { - await queryClient.resetQueries({ queryKey: consoleQuery.account.profile.get.key() }) - const redirectUrl = resolvePostLoginRedirect(searchParams) - router.replace(redirectUrl || '/') + const profileQueryOptions = userProfileQueryOptions() + await queryClient.resetQueries({ queryKey: profileQueryOptions.queryKey }) + await queryClient.fetchQuery(profileQueryOptions) + replaceLoginRedirect(resolvePostLoginRedirect(searchParams), router.replace, basePath) } } } catch (error) { diff --git a/web/app/signin/components/mail-and-password-auth.tsx b/web/app/signin/components/mail-and-password-auth.tsx index baa33938479..82c575347de 100644 --- a/web/app/signin/components/mail-and-password-auth.tsx +++ b/web/app/signin/components/mail-and-password-auth.tsx @@ -14,6 +14,8 @@ import { consoleQuery } from '@/service/client' import { login } from '@/service/common' import { setWebAppAccessToken } from '@/service/webapp-auth' import { encryptPassword } from '@/utils/encryption' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' import { resolvePostLoginRedirect } from '../utils/post-login-redirect' type MailAndPasswordAuthProps = { @@ -88,8 +90,7 @@ export default function MailAndPasswordAuth({ isInvite, isEmailSetup }: MailAndP router.replace(`/signin/invite-settings?${searchParams.toString()}`) } else { await queryClient.resetQueries({ queryKey: consoleQuery.account.profile.get.key() }) - const redirectUrl = resolvePostLoginRedirect(searchParams) - router.replace(redirectUrl || '/') + replaceLoginRedirect(resolvePostLoginRedirect(searchParams), router.replace, basePath) } } else { toast.error(res.data) diff --git a/web/app/signin/invite-settings/__tests__/page.spec.tsx b/web/app/signin/invite-settings/__tests__/page.spec.tsx index 86f35ac54e9..f76974a11fa 100644 --- a/web/app/signin/invite-settings/__tests__/page.spec.tsx +++ b/web/app/signin/invite-settings/__tests__/page.spec.tsx @@ -58,10 +58,6 @@ vi.mock('@/utils/timezone', () => ({ ], })) -vi.mock('../utils/post-login-redirect', () => ({ - resolvePostLoginRedirect: vi.fn(() => null), -})) - const mockReplace = vi.fn() const mockRefetch = vi.fn() @@ -243,4 +239,25 @@ describe('InviteSettingsPage', () => { }) }) }) + + describe('Post-activation redirect', () => { + it('should use the console home when the redirect target is external', async () => { + mockUseSearchParams.mockReturnValue( + new URLSearchParams( + 'invite_token=invite-token&redirect_url=https%3A%2F%2Fgoogle.com', + ) as unknown as ReturnType, + ) + + render() + + fireEvent.change(screen.getByLabelText('login.name'), { + target: { value: 'Invitee' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'login.join Acme' })) + + await waitFor(() => { + expect(mockReplace).toHaveBeenCalledWith('/') + }) + }) + }) }) diff --git a/web/app/signin/invite-settings/page.tsx b/web/app/signin/invite-settings/page.tsx index 8c5024ae3f2..41de03fa7b4 100644 --- a/web/app/signin/invite-settings/page.tsx +++ b/web/app/signin/invite-settings/page.tsx @@ -27,7 +27,9 @@ import { useRouter, useSearchParams } from '@/next/navigation' import { consoleQuery } from '@/service/client' import { activateMember } from '@/service/common' import { useInvitationCheck } from '@/service/use-common' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' import { getBrowserTimezone, timezones } from '@/utils/timezone' +import { basePath } from '@/utils/var' import { resolvePostLoginRedirect } from '../utils/post-login-redirect' type LanguageSelectOption = { @@ -118,8 +120,7 @@ export default function InviteSettingsPage() { // Tokens are now stored in cookies by the backend if (requiresAccountSetup) await setLocaleOnClient(language!, false) await queryClient.resetQueries({ queryKey: consoleQuery.account.profile.get.key() }) - const redirectUrl = resolvePostLoginRedirect(searchParams) - router.replace(redirectUrl || '/') + replaceLoginRedirect(resolvePostLoginRedirect(searchParams), router.replace, basePath) } } catch { recheck() diff --git a/web/app/signin/normal-form.tsx b/web/app/signin/normal-form.tsx index 4af4a4b954d..d6e3822aaf5 100644 --- a/web/app/signin/normal-form.tsx +++ b/web/app/signin/normal-form.tsx @@ -11,6 +11,8 @@ import { LicenseStatus } from '@/features/system-features/constants' import Link from '@/next/link' import { useRouter, useSearchParams } from '@/next/navigation' import { invitationCheck } from '@/service/common' +import { replaceLoginRedirect } from '@/utils/login-redirect.client' +import { basePath } from '@/utils/var' import Loading from '../components/base/loading' import MailAndCodeAuth from './components/mail-and-code-auth' import MailAndPasswordAuth from './components/mail-and-password-auth' @@ -89,8 +91,7 @@ function NormalForm() { return } - const redirectUrl = resolvePostLoginRedirect(searchParams) - router.replace(redirectUrl || '/') + replaceLoginRedirect(resolvePostLoginRedirect(searchParams), router.replace, basePath) }, [isInviteLink, isLoggedIn, router, searchParams]) useEffect(() => { diff --git a/web/app/signin/utils/__tests__/post-login-redirect.spec.ts b/web/app/signin/utils/__tests__/post-login-redirect.spec.ts index 5a5c26570c6..995128c183e 100644 --- a/web/app/signin/utils/__tests__/post-login-redirect.spec.ts +++ b/web/app/signin/utils/__tests__/post-login-redirect.spec.ts @@ -3,8 +3,8 @@ import { resolvePostLoginRedirect, setPostLoginRedirect } from '../post-login-re describe('post-login redirect utilities', () => { beforeEach(() => { + vi.clearAllMocks() vi.useRealTimers() - window.localStorage.clear() window.sessionStorage.clear() }) @@ -19,21 +19,94 @@ describe('post-login redirect utilities', () => { resolvePostLoginRedirect( searchParams as unknown as Parameters[0], ), - ).toBe( - '/account/oauth/authorize?client_id=app&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback', - ) + ).toEqual({ + kind: 'internal', + href: '/account/oauth/authorize?client_id=app&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback', + }) + }) + + it('should allow a trusted Dify absolute query target', () => { + const searchParams = new URLSearchParams({ + redirect_url: 'https://docs.eu.dify.ai/getting-started', + }) + + expect( + resolvePostLoginRedirect( + searchParams as unknown as Parameters[0], + ), + ).toEqual({ kind: 'absolute', href: 'https://docs.eu.dify.ai/getting-started' }) + }) + + it('should allow an absolute target on the current self-hosted origin', () => { + const redirectUrl = `${window.location.origin}/apps` + const searchParams = new URLSearchParams({ redirect_url: redirectUrl }) + + expect( + resolvePostLoginRedirect( + searchParams as unknown as Parameters[0], + ), + ).toEqual({ kind: 'absolute', href: redirectUrl }) + }) + + it('should use the default target instead of a stored device target when the query target is invalid', () => { + setPostLoginRedirect('/device?user_code=ABCD') + const searchParams = new URLSearchParams({ redirect_url: 'https://google.com' }) + + expect( + resolvePostLoginRedirect( + searchParams as unknown as Parameters[0], + ), + ).toEqual({ kind: 'internal', href: '/' }) + expect(resolvePostLoginRedirect()).toEqual({ + kind: 'internal', + href: '/device?user_code=ABCD', + }) + }) + + it('should treat an empty redirect_url query param as invalid instead of using device storage', () => { + setPostLoginRedirect('/device?user_code=ABCD') + const searchParams = new URLSearchParams('redirect_url=') + + expect( + resolvePostLoginRedirect( + searchParams as unknown as Parameters[0], + ), + ).toEqual({ kind: 'internal', href: '/' }) + expect(resolvePostLoginRedirect()).toEqual({ + kind: 'internal', + href: '/device?user_code=ABCD', + }) }) it('should recover a valid device redirect from sessionStorage once', () => { setPostLoginRedirect('/device?user_code=ABCD&sso_verified=true') - expect(resolvePostLoginRedirect()).toBe('/device?user_code=ABCD&sso_verified=true') - expect(resolvePostLoginRedirect()).toBeNull() + expect(resolvePostLoginRedirect()).toEqual({ + kind: 'internal', + href: '/device?user_code=ABCD&sso_verified=true', + }) + expect(resolvePostLoginRedirect()).toEqual({ kind: 'internal', href: '/' }) + }) + + it('should discard an expired device redirect', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-07-13T00:00:00Z')) + setPostLoginRedirect('/device?user_code=ABCD') + vi.advanceTimersByTime(15 * 60 * 1000 + 1) + + expect(resolvePostLoginRedirect()).toEqual({ kind: 'internal', href: '/' }) }) it('should ignore invalid stored redirects', () => { setPostLoginRedirect('https://example.com/device?user_code=ABCD') - expect(resolvePostLoginRedirect()).toBeNull() + expect(resolvePostLoginRedirect()).toEqual({ kind: 'internal', href: '/' }) + }) + + it('should preserve the device path and query-key allowlist', () => { + setPostLoginRedirect('/device?user_code=ABCD&next=/apps') + setPostLoginRedirect('/apps') + + expect(resolvePostLoginRedirect()).toEqual({ kind: 'internal', href: '/' }) }) }) diff --git a/web/app/signin/utils/post-login-redirect.ts b/web/app/signin/utils/post-login-redirect.ts index 21477cbf583..8bffa75e4eb 100644 --- a/web/app/signin/utils/post-login-redirect.ts +++ b/web/app/signin/utils/post-login-redirect.ts @@ -1,4 +1,6 @@ import type { ReadonlyURLSearchParams } from '@/next/navigation' +import { IS_CLOUD_EDITION } from '@/config' +import { getClientLoginFallback, resolveLoginRedirectTarget } from '@/utils/login-redirect' const REDIRECT_URL_KEY = 'redirect_url' const DEVICE_REDIRECT_KEY = 'dify_post_login_redirect' @@ -9,10 +11,17 @@ const ALLOWED: Record> = { '/account/oauth/authorize': new Set(['client_id', 'scope', 'state', 'redirect_uri']), } -function validate(target: string): string | null { +function validateDeviceRedirect(target: string): string | null { if (typeof window === 'undefined') return null + try { - const url = new URL(target, window.location.origin) + const safeTarget = resolveLoginRedirectTarget(target, { + allowSameOriginAbsolute: true, + currentOrigin: window.location.origin, + }) + if (!safeTarget) return null + + const url = new URL(safeTarget.href, window.location.origin) if (url.origin !== window.location.origin) return null const allowedKeys = ALLOWED[url.pathname] if (!allowedKeys) return null @@ -29,7 +38,7 @@ function validate(target: string): string | null { // OAuth, SSO IdP bounce). sessionStorage is tab-scoped so concurrent // /device tabs don't clobber each other. 15-min TTL drops stale values. // Same-origin + exact-path whitelist prevents open-redirect. -export const setPostLoginRedirect = (value: string | null) => { +export function setPostLoginRedirect(value: string | null) { if (typeof window === 'undefined') return if (value === null) { try { @@ -37,7 +46,7 @@ export const setPostLoginRedirect = (value: string | null) => { } catch {} return } - const safe = validate(value) + const safe = validateDeviceRedirect(value) if (!safe) return try { sessionStorage.setItem(DEVICE_REDIRECT_KEY, JSON.stringify({ target: safe, ts: Date.now() })) @@ -58,24 +67,26 @@ function getDeviceRedirect(): string | null { const parsed = JSON.parse(raw) if (typeof parsed?.target !== 'string' || typeof parsed?.ts !== 'number') return null if (Date.now() - parsed.ts > DEVICE_TTL_MS) return null - return validate(parsed.target) + return validateDeviceRedirect(parsed.target) } catch { return null } } -export const resolvePostLoginRedirect = (searchParams?: ReadonlyURLSearchParams) => { - if (searchParams) { - const redirectUrl = searchParams.get(REDIRECT_URL_KEY) - if (redirectUrl) { - try { - return decodeURIComponent(redirectUrl) - } catch { - return redirectUrl - } - } +export function resolvePostLoginRedirect(searchParams?: ReadonlyURLSearchParams) { + const currentOrigin = typeof window === 'undefined' ? undefined : window.location.origin + const fallback = getClientLoginFallback(IS_CLOUD_EDITION) + + if (searchParams?.has(REDIRECT_URL_KEY)) { + return ( + resolveLoginRedirectTarget(searchParams.get(REDIRECT_URL_KEY), { + allowSameOriginAbsolute: true, + currentOrigin, + }) ?? fallback + ) } + const device = getDeviceRedirect() - if (device) return device - return null + if (device) return { kind: 'internal' as const, href: device } + return fallback } diff --git a/web/context/web-app-context.tsx b/web/context/web-app-context.tsx index e532c1b2cf7..2c9f26b5a79 100644 --- a/web/context/web-app-context.tsx +++ b/web/context/web-app-context.tsx @@ -6,6 +6,10 @@ import type { AppData, AppMeta } from '@/models/share' import { useQuery } from '@tanstack/react-query' import { useEffect } from 'react' import { create } from 'zustand' +import { + isWebAppSigninPath, + resolveWebAppLoginRedirect, +} from '@/app/(shareLayout)/webapp-signin/login-redirect' import { getProcessedSystemVariablesFromUrlParams } from '@/app/components/base/chat/utils' import Loading from '@/app/components/base/loading' import { systemFeaturesQueryOptions } from '@/features/system-features/client' @@ -53,13 +57,8 @@ export const useWebAppStore = create((set) => ({ })) const getShareCodeFromRedirectUrl = (redirectUrl: string | null): string | null => { - if (!redirectUrl || redirectUrl.length === 0) return null - try { - const url = new URL(decodeURIComponent(redirectUrl), 'https://dify.local') - return url.pathname.split('/').pop() || null - } catch { - return null - } + const currentOrigin = typeof window === 'undefined' ? undefined : window.location.origin + return resolveWebAppLoginRedirect(redirectUrl, currentOrigin)?.appCode || null } const getShareCodeFromPathname = (pathname: string): string | null => { const code = pathname.split('/').pop() || null @@ -79,8 +78,9 @@ const WebAppStoreProvider: FC = ({ children }) => { const searchParamsString = searchParams.toString() // Compute shareCode directly + const redirectShareCode = getShareCodeFromRedirectUrl(redirectUrlParam) const shareCode = - getShareCodeFromRedirectUrl(redirectUrlParam) || getShareCodeFromPathname(pathname) + redirectShareCode || (isWebAppSigninPath(pathname) ? null : getShareCodeFromPathname(pathname)) useEffect(() => { updateShareCode(shareCode) }, [shareCode, updateShareCode]) diff --git a/web/service/__tests__/base.spec.ts b/web/service/__tests__/base.spec.ts index a4d1dcbfe7c..748de185df2 100644 --- a/web/service/__tests__/base.spec.ts +++ b/web/service/__tests__/base.spec.ts @@ -1,4 +1,8 @@ -import { buildSigninUrlWithRedirect } from '../base' +import { + buildSigninUrlWithRedirect, + buildWebAppSigninUrlWithRedirect, + isWebAppSigninPath, +} from '../base' vi.mock('@/utils/var', () => ({ basePath: '/app', @@ -66,3 +70,34 @@ describe('buildSigninUrlWithRedirect', () => { expect(url).toBe('https://example.com/app/signin') }) }) + +describe('buildWebAppSigninUrlWithRedirect', () => { + it('should encode the internal redirect target exactly once', () => { + const url = buildWebAppSigninUrlWithRedirect( + 'https://example.com', + '/chatbot/share-app', + '?foo=bar', + ) + + expect(url).toBe( + 'https://example.com/app/webapp-signin?redirect_url=%2Fchatbot%2Fshare-app%3Ffoo%3Dbar', + ) + expect(new URL(url).searchParams.get('redirect_url')).toBe('/chatbot/share-app?foo=bar') + }) +}) + +describe('isWebAppSigninPath', () => { + it.each(['/app/webapp-signin', '/app/webapp-signin/'])( + 'should recognize the web app signin route behind basePath: %s', + (pathname) => { + expect(isWebAppSigninPath(pathname)).toBe(true) + }, + ) + + it.each(['/webapp-signin', '/app/webapp-signin-extra', '/app/webapp-signin/check-code'])( + 'should not match a different path: %s', + (pathname) => { + expect(isWebAppSigninPath(pathname)).toBe(false) + }, + ) +}) diff --git a/web/service/base.ts b/web/service/base.ts index 8c73f5715b3..c47fcff5f7d 100644 --- a/web/service/base.ts +++ b/web/service/base.ts @@ -184,20 +184,41 @@ function unicodeToChar(text: string) { } const WBB_APP_LOGIN_PATH = '/webapp-signin' + +export function isWebAppSigninPath(pathname: string) { + const basePathSegment = basePath.replace(/^\/+|\/+$/g, '') + const signinPath = `${basePathSegment ? `/${basePathSegment}` : ''}${WBB_APP_LOGIN_PATH}` + return pathname === signinPath || pathname === `${signinPath}/` +} + +export function buildWebAppSigninUrlWithRedirect( + origin: string, + pathname: string, + search: string, + message?: string, + code?: number, +) { + const params = new URLSearchParams() + params.set('redirect_url', `${pathname}${search}`) + if (message) params.set('message', message) + if (code) params.set('code', String(code)) + + return `${origin}${basePath}${WBB_APP_LOGIN_PATH}?${params.toString()}` +} + function requiredWebSSOLogin(message?: string, code?: number) { if (!isClient) return - const params = new URLSearchParams() // prevent redirect loop - if (window.location.pathname === WBB_APP_LOGIN_PATH) return + if (isWebAppSigninPath(window.location.pathname)) return - params.append( - 'redirect_url', - encodeURIComponent(`${window.location.pathname}${window.location.search}`), + window.location.href = buildWebAppSigninUrlWithRedirect( + window.location.origin, + window.location.pathname, + window.location.search, + message, + code, ) - if (message) params.append('message', message) - if (code) params.append('code', String(code)) - window.location.href = `${window.location.origin}${basePath}${WBB_APP_LOGIN_PATH}?${params.toString()}` } function formatURL(url: string, isPublicAPI: boolean) { diff --git a/web/utils/__tests__/login-redirect.client.spec.ts b/web/utils/__tests__/login-redirect.client.spec.ts new file mode 100644 index 00000000000..8bccf01062d --- /dev/null +++ b/web/utils/__tests__/login-redirect.client.spec.ts @@ -0,0 +1,50 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { replaceLoginRedirect } from '../login-redirect.client' + +describe('replaceLoginRedirect', () => { + const routerReplace = vi.fn() + const locationReplace = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + vi.stubGlobal('location', { + ...window.location, + replace: locationReplace, + } as unknown as Location) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it.each([ + ['/', '', '/'], + ['/apps?tag=agent#latest', '/console', '/apps?tag=agent#latest'], + ['/console/apps?tag=agent#latest', '/console', '/apps?tag=agent#latest'], + ['/console?tag=agent#latest', '/console/', '/?tag=agent#latest'], + ['/console-apps', '/console', '/console-apps'], + ['/console//evil.example', '/console', '/'], + ['/console/%2Fevil.example', '/console', '/'], + ['/console/%252Fevil.example', '/console', '/'], + ['/console/\\evil.example', '/console', '/'], + ])( + 'should navigate the internal target %s with basePath %s as %s', + (href, basePath, expected) => { + replaceLoginRedirect({ kind: 'internal', href }, routerReplace, basePath) + + expect(routerReplace).toHaveBeenCalledWith(expected) + expect(locationReplace).not.toHaveBeenCalled() + }, + ) + + it('should replace the browser location for an absolute target', () => { + replaceLoginRedirect( + { kind: 'absolute', href: 'https://cloud.dify.ai/apps' }, + routerReplace, + '/console', + ) + + expect(locationReplace).toHaveBeenCalledWith('https://cloud.dify.ai/apps') + expect(routerReplace).not.toHaveBeenCalled() + }) +}) diff --git a/web/utils/__tests__/login-redirect.spec.ts b/web/utils/__tests__/login-redirect.spec.ts new file mode 100644 index 00000000000..56d41cbc16a --- /dev/null +++ b/web/utils/__tests__/login-redirect.spec.ts @@ -0,0 +1,172 @@ +import { describe, expect, it } from 'vitest' +import { + getClientLoginFallback, + getServerLoginFallback, + resolveLoginRedirectTarget, +} from '../login-redirect' + +const currentOrigin = 'http://localhost:3000' + +function resolve(raw: string | null | undefined) { + return resolveLoginRedirectTarget(raw, { + allowSameOriginAbsolute: true, + currentOrigin, + }) +} + +describe('resolveLoginRedirectTarget', () => { + describe('allowed targets', () => { + it.each([ + ['/apps', { kind: 'internal', href: '/apps' }], + ['/apps?tag=agent#latest', { kind: 'internal', href: '/apps?tag=agent#latest' }], + ['/discount/100%25', { kind: 'internal', href: '/discount/100%25' }], + ['/files/%2525', { kind: 'internal', href: '/files/%2525' }], + [ + '/account/oauth/authorize?client_id=app&redirect_uri=https%3A%2F%2Fclient.example%2Fcallback', + { + kind: 'internal', + href: '/account/oauth/authorize?client_id=app&redirect_uri=https%3A%2F%2Fclient.example%2Fcallback', + }, + ], + ])('should allow the internal target %s', (raw, expected) => { + expect(resolve(raw)).toEqual(expected) + }) + + it.each(['http://localhost:3000/apps', 'http://localhost:3000/apps?tag=agent#latest'])( + 'should allow the same-origin absolute target %s', + (raw) => { + expect(resolve(raw)).toEqual({ kind: 'absolute', href: raw }) + }, + ) + + it.each([ + ['https://dify.ai', 'https://dify.ai/'], + ['https://cloud.dify.ai/apps', 'https://cloud.dify.ai/apps'], + [ + 'https://docs.eu.dify.ai/path?from=login#section', + 'https://docs.eu.dify.ai/path?from=login#section', + ], + ['https://dify.ai:443/apps', 'https://dify.ai/apps'], + ])('should allow the trusted Dify target %s', (raw, expected) => { + expect(resolve(raw)).toEqual({ kind: 'absolute', href: expected }) + }) + + it('should allow a same-origin custom HTTPS port', () => { + expect( + resolveLoginRedirectTarget('https://self-hosted.example:8443/apps', { + allowSameOriginAbsolute: true, + currentOrigin: 'https://self-hosted.example:8443', + }), + ).toEqual({ kind: 'absolute', href: 'https://self-hosted.example:8443/apps' }) + }) + }) + + describe('legacy encoding compatibility', () => { + it.each([ + ['%2Fapps%3Ftag%3Dagent%23latest', { kind: 'internal', href: '/apps?tag=agent#latest' }], + [ + 'https%3A%2F%2Fcloud.dify.ai%2Fapps', + { kind: 'absolute', href: 'https://cloud.dify.ai/apps' }, + ], + ])('should decode the legacy target %s once', (raw, expected) => { + expect(resolve(raw)).toEqual(expected) + }) + + it('should preserve an encoded nested OAuth callback when the top-level target is already valid', () => { + const raw = + '/account/oauth/authorize?client_id=app&redirect_uri=https%3A%2F%2Fclient.example%2Fcallback%3Fnext%3D%252Fdone' + + expect(resolve(raw)).toEqual({ kind: 'internal', href: raw }) + }) + }) + + describe('rejected targets', () => { + it.each([ + null, + undefined, + '', + ' ', + 'apps', + 'https://google.com', + 'http://cloud.dify.ai', + 'https://cloud.dify.ai:444/apps', + 'https://dify.ai.evil.example', + 'https://evildify.ai', + 'https://dify.ai@evil.example', + 'https://user:password@dify.ai', + 'javascript:alert(1)', + 'data:text/html,hello', + 'blob:http://localhost:3000/redirect', + '//evil.example', + '///evil.example', + '\\\\evil.example', + '/\\evil.example', + '/path\\to\\resource', + 'https://cloud.dify.ai//evil.example', + 'http://localhost:3000//evil.example', + '/%2Fevil.example', + '/%252Fevil.example', + '/%5Cevil.example', + '/%255Cevil.example', + '/%252e%252e//evil.example', + '%2F%2Fevil.example', + '%252F%252Fevil.example', + '%5C%5Cevil.example', + '%255C%255Cevil.example', + '/apps%ZZ', + '/apps%252G', + '/%2500evil.example', + ' https://dify.ai', + 'https://dify.ai\n.evil.example', + ])('should reject %s', (raw) => { + expect(resolve(raw)).toBeNull() + }) + + it('should reject a same-origin absolute URL when same-origin absolute targets are disabled', () => { + expect( + resolveLoginRedirectTarget('http://localhost:3000/apps', { + allowSameOriginAbsolute: false, + currentOrigin, + }), + ).toBeNull() + }) + + it('should reject a malformed current origin without throwing', () => { + expect( + resolveLoginRedirectTarget('https://self-hosted.example/apps', { + allowSameOriginAbsolute: true, + currentOrigin: 'not an origin', + }), + ).toBeNull() + }) + }) +}) + +describe('login redirect fallbacks', () => { + it('should use the Cloud console home for the client Cloud fallback', () => { + expect(getClientLoginFallback(true)).toEqual({ + kind: 'absolute', + href: 'https://cloud.dify.ai/', + }) + }) + + it('should use an internal root for the client self-hosted fallback', () => { + expect(getClientLoginFallback(false)).toEqual({ kind: 'internal', href: '/' }) + }) + + it.each([ + ['', '/'], + ['/', '/'], + ['/console', '/console/'], + ['/console/', '/console/'], + ])('should include basePath %s in the server self-hosted fallback', (basePath, href) => { + expect(getServerLoginFallback(false, basePath)).toEqual({ kind: 'internal', href }) + }) + + it('should use the Cloud console home for the server Cloud fallback', () => { + expect(getServerLoginFallback(true, '/console')).toEqual({ + kind: 'absolute', + href: 'https://cloud.dify.ai/', + }) + }) +}) diff --git a/web/utils/login-redirect.client.ts b/web/utils/login-redirect.client.ts new file mode 100644 index 00000000000..0957b14392b --- /dev/null +++ b/web/utils/login-redirect.client.ts @@ -0,0 +1,40 @@ +'use client' + +import type { LoginRedirectTarget } from './login-redirect' +import { resolveLoginRedirectTarget } from './login-redirect' + +type RouterReplace = (href: string) => void + +function normalizeBasePath(value: string) { + if (!value || value === '/') return '' + const withLeadingSlash = value.startsWith('/') ? value : `/${value}` + return withLeadingSlash.replace(/\/+$/, '') +} + +function removeBasePathOnce(href: string, basePath: string) { + const normalizedBasePath = normalizeBasePath(basePath) + if (!normalizedBasePath) return href + + const url = new URL(href, 'https://login-redirect.invalid') + let pathname = url.pathname + if (pathname === normalizedBasePath) pathname = '/' + else if (pathname.startsWith(`${normalizedBasePath}/`)) + pathname = pathname.slice(normalizedBasePath.length) || '/' + + return `${pathname}${url.search}${url.hash}` +} + +export function replaceLoginRedirect( + target: LoginRedirectTarget, + routerReplace: RouterReplace, + basePath: string, +) { + if (target.kind === 'absolute') { + globalThis.location.replace(target.href) + return + } + + const hrefWithoutBasePath = removeBasePathOnce(target.href, basePath) + const safeTarget = resolveLoginRedirectTarget(hrefWithoutBasePath) + routerReplace(safeTarget?.kind === 'internal' ? safeTarget.href : '/') +} diff --git a/web/utils/login-redirect.ts b/web/utils/login-redirect.ts new file mode 100644 index 00000000000..c9271ec3041 --- /dev/null +++ b/web/utils/login-redirect.ts @@ -0,0 +1,192 @@ +export type LoginRedirectTarget = + | { kind: 'internal'; href: string } + | { kind: 'absolute'; href: string } + +type ResolveLoginRedirectOptions = { + currentOrigin?: string + allowSameOriginAbsolute?: boolean +} + +const CLOUD_CONSOLE_HOME = 'https://cloud.dify.ai/' +const INTERNAL_URL_BASE = 'https://login-redirect.invalid' +const CONTROL_CHARACTER_PATTERN = /[\u0000-\u001F\u007F]/ +const MALFORMED_PERCENT_PATTERN = /%(?![\dA-F]{2})/i +const HEX_CHARACTER_PATTERN = /^[\dA-F]$/i + +function isSingleSlashPath(pathname: string) { + return pathname.startsWith('/') && !pathname.startsWith('//') && !pathname.startsWith('/\\') +} + +function decodePathnameLayer(pathname: string, allowLiteralPercent: boolean) { + let decodablePathname = '' + + for (let index = 0; index < pathname.length; index += 1) { + const character = pathname[index] + if (character !== '%') { + decodablePathname += character + continue + } + + const firstHexCharacter = pathname[index + 1] + const secondHexCharacter = pathname[index + 2] + const hasFirstHexCharacter = HEX_CHARACTER_PATTERN.test(firstHexCharacter || '') + const hasSecondHexCharacter = HEX_CHARACTER_PATTERN.test(secondHexCharacter || '') + + if (hasFirstHexCharacter && hasSecondHexCharacter) { + decodablePathname += pathname.slice(index, index + 3) + index += 2 + continue + } + + if (!allowLiteralPercent || hasFirstHexCharacter) return null + decodablePathname += '%25' + } + + try { + return decodeURIComponent(decodablePathname) + } catch { + return null + } +} + +function hasSafePathname(pathname: string) { + let decodedPathname = pathname + + for (let decodeCount = 0; decodeCount <= 2; decodeCount += 1) { + if ( + !isSingleSlashPath(decodedPathname) || + decodedPathname.includes('\\') || + CONTROL_CHARACTER_PATTERN.test(decodedPathname) + ) + return false + + try { + const normalizedUrl = new URL(decodedPathname, INTERNAL_URL_BASE) + if ( + normalizedUrl.origin !== INTERNAL_URL_BASE || + !isSingleSlashPath(normalizedUrl.pathname) || + normalizedUrl.pathname.includes('\\') + ) + return false + } catch { + return false + } + + if (decodeCount === 2) return true + + const nextPathname = decodePathnameLayer(decodedPathname, decodeCount > 0) + if (nextPathname === null) return false + if (nextPathname === decodedPathname) return true + decodedPathname = nextPathname + } + + return false +} + +function getHTTPOrigin(value: string | undefined) { + if (!value) return null + + try { + const url = new URL(value) + if (url.protocol !== 'http:' && url.protocol !== 'https:') return null + return url.origin + } catch { + return null + } +} + +function isTrustedDifyHostname(hostname: string) { + return hostname === 'dify.ai' || hostname.endsWith('.dify.ai') +} + +function validateLoginRedirectTarget( + candidate: string, + options: ResolveLoginRedirectOptions, +): LoginRedirectTarget | null { + if ( + candidate.length === 0 || + candidate !== candidate.trim() || + CONTROL_CHARACTER_PATTERN.test(candidate) || + MALFORMED_PERCENT_PATTERN.test(candidate) || + candidate.includes('\\') + ) + return null + + if (candidate.startsWith('/')) { + if (!isSingleSlashPath(candidate)) return null + + try { + const url = new URL(candidate, INTERNAL_URL_BASE) + if (url.origin !== INTERNAL_URL_BASE || !hasSafePathname(url.pathname)) return null + + const href = `${url.pathname}${url.search}${url.hash}` + if (!isSingleSlashPath(href)) return null + return { kind: 'internal', href } + } catch { + return null + } + } + + let url: URL + try { + url = new URL(candidate) + } catch { + return null + } + + if ( + (url.protocol !== 'http:' && url.protocol !== 'https:') || + url.username !== '' || + url.password !== '' || + !hasSafePathname(url.pathname) + ) + return null + + const currentOrigin = getHTTPOrigin(options.currentOrigin) + if (options.allowSameOriginAbsolute && currentOrigin && url.origin === currentOrigin) + return { kind: 'absolute', href: url.href } + + if (url.protocol !== 'https:' || url.port !== '' || !isTrustedDifyHostname(url.hostname)) + return null + + return { kind: 'absolute', href: url.href } +} + +function normalizeBasePath(value: string) { + if (!value || value === '/') return '' + const withLeadingSlash = value.startsWith('/') ? value : `/${value}` + return withLeadingSlash.replace(/\/+$/, '') +} + +export function resolveLoginRedirectTarget( + raw: string | null | undefined, + options: ResolveLoginRedirectOptions = {}, +): LoginRedirectTarget | null { + if (!raw) return null + + const target = validateLoginRedirectTarget(raw, options) + if (target) return target + + try { + const decodedTarget = decodeURIComponent(raw) + if (decodedTarget === raw) return null + return validateLoginRedirectTarget(decodedTarget, options) + } catch { + return null + } +} + +export function getClientLoginFallback(isCloudEdition: boolean): LoginRedirectTarget { + if (isCloudEdition) return { kind: 'absolute', href: CLOUD_CONSOLE_HOME } + return { kind: 'internal', href: '/' } +} + +export function getServerLoginFallback( + isCloudEdition: boolean, + basePath: string, +): LoginRedirectTarget { + if (isCloudEdition) return { kind: 'absolute', href: CLOUD_CONSOLE_HOME } + + const normalizedBasePath = normalizeBasePath(basePath) + return { kind: 'internal', href: normalizedBasePath ? `${normalizedBasePath}/` : '/' } +}