diff --git a/web/app/account/oauth/authorize/page.spec.tsx b/web/app/account/oauth/authorize/page.spec.tsx new file mode 100644 index 0000000000..3b367710eb --- /dev/null +++ b/web/app/account/oauth/authorize/page.spec.tsx @@ -0,0 +1,127 @@ +import { fireEvent, render, screen } from '@testing-library/react' +import { useRouter, useSearchParams } from 'next/navigation' +import { useLanguage } from '@/app/components/header/account-setting/model-provider-page/hooks' +import { useAppContext } from '@/context/app-context' +import { useIsLogin } from '@/service/use-common' +import { useAuthorizeOAuthApp, useOAuthAppInfo } from '@/service/use-oauth' +import { storage } from '@/utils/storage' +import { OAUTH_AUTHORIZE_PENDING_KEY, OAUTH_AUTHORIZE_PENDING_TTL, REDIRECT_URL_KEY } from './constants' +import OAuthAuthorize from './page' + +vi.mock('next/navigation', () => ({ + useRouter: vi.fn(), + useSearchParams: vi.fn(), +})) + +vi.mock('@/app/components/header/account-setting/model-provider-page/hooks', () => ({ + useLanguage: vi.fn(), +})) + +vi.mock('@/context/app-context', () => ({ + useAppContext: vi.fn(), +})) + +vi.mock('@/service/use-common', () => ({ + useIsLogin: vi.fn(), +})) + +vi.mock('@/service/use-oauth', () => ({ + useAuthorizeOAuthApp: vi.fn(), + useOAuthAppInfo: vi.fn(), +})) + +const FIXED_DATE = new Date('2026-02-10T12:00:00.000Z') +const SEARCH_QUERY = 'client_id=dcfcd6a4-5799-405a-a6d7-04261b24dd02&redirect_uri=https%3A%2F%2Fcreators.dify.dev%2Fapi%2Fv1%2Foauth%2Fcallback%2Fdify&response_type=code' + +const createOAuthAppInfo = () => ({ + app_label: { + en_US: 'Test OAuth App', + }, + scope: 'read:name', + app_icon: '', +}) + +describe('OAuthAuthorize redirect persistence', () => { + const push = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + storage.resetCache() + localStorage.clear() + vi.useFakeTimers() + vi.setSystemTime(FIXED_DATE) + + vi.mocked(useRouter).mockReturnValue({ + push, + } as never) + vi.mocked(useSearchParams).mockReturnValue(new URLSearchParams(SEARCH_QUERY) as never) + vi.mocked(useLanguage).mockReturnValue('en_US') + vi.mocked(useAppContext).mockReturnValue({ + userProfile: { + avatar_url: '', + name: 'Dify User', + email: 'dify@example.com', + }, + } as never) + vi.mocked(useOAuthAppInfo).mockReturnValue({ + data: createOAuthAppInfo(), + isLoading: false, + isError: false, + } as never) + vi.mocked(useAuthorizeOAuthApp).mockReturnValue({ + mutateAsync: vi.fn(), + isPending: false, + } as never) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('should store full authorize url and navigate to signin when switch account is clicked', () => { + // Arrange + vi.mocked(useIsLogin).mockReturnValue({ + isLoading: false, + data: { logged_in: true }, + } as never) + render() + const switchAccountButton = screen.getByRole('button', { name: 'oauth.switchAccount' }) + + // Act + fireEvent.click(switchAccountButton) + + // Assert + const expectedStoredReturnUrl = `${window.location.origin}/account/oauth/authorize?${SEARCH_QUERY}` + const expectedDecodedReturnUrl = decodeURIComponent(expectedStoredReturnUrl) + expect(push).toHaveBeenCalledTimes(1) + const pushedUrl = push.mock.calls[0][0] as string + const pushedParams = new URLSearchParams(pushedUrl.split('?')[1]) + expect(pushedParams.has(REDIRECT_URL_KEY)).toBe(true) + expect(decodeURIComponent(pushedParams.get(REDIRECT_URL_KEY)!)).toBe(expectedDecodedReturnUrl) + + const storedPendingRedirect = storage.get<{ value: string, expiry: number }>(OAUTH_AUTHORIZE_PENDING_KEY) + expect(storedPendingRedirect).toEqual({ + value: expectedStoredReturnUrl, + expiry: Math.floor((FIXED_DATE.getTime() + OAUTH_AUTHORIZE_PENDING_TTL * 1000) / 1000), + }) + }) + + it('should store full authorize url and navigate to signin when login button is clicked for logged-out users', () => { + // Arrange + vi.mocked(useIsLogin).mockReturnValue({ + isLoading: false, + data: { logged_in: false }, + } as never) + render() + const loginButton = screen.getByRole('button', { name: 'oauth.login' }) + + // Act + fireEvent.click(loginButton) + + // Assert + const expectedReturnUrl = `${window.location.origin}/account/oauth/authorize?${SEARCH_QUERY}` + expect(push).toHaveBeenCalledTimes(1) + expect(push).toHaveBeenCalledWith(`/signin?${REDIRECT_URL_KEY}=${encodeURIComponent(expectedReturnUrl)}`) + expect(storage.get<{ value: string }>(OAUTH_AUTHORIZE_PENDING_KEY)?.value).toBe(expectedReturnUrl) + }) +}) diff --git a/web/app/account/oauth/authorize/page.tsx b/web/app/account/oauth/authorize/page.tsx index ea375e1a32..bc7cd1668e 100644 --- a/web/app/account/oauth/authorize/page.tsx +++ b/web/app/account/oauth/authorize/page.tsx @@ -85,7 +85,8 @@ export default function OAuthAuthorize() { const isLoading = isOAuthLoading || isIsLoginLoading const onLoginSwitchClick = () => { try { - const returnUrl = buildReturnUrl('/account/oauth/authorize', `?client_id=${encodeURIComponent(client_id)}&redirect_uri=${encodeURIComponent(redirect_uri)}`) + const authorizeQuery = searchParams.toString() + const returnUrl = buildReturnUrl('/account/oauth/authorize', authorizeQuery ? `?${authorizeQuery}` : '') setItemWithExpiry(OAUTH_AUTHORIZE_PENDING_KEY, returnUrl, OAUTH_AUTHORIZE_PENDING_TTL) router.push(`/signin?${REDIRECT_URL_KEY}=${encodeURIComponent(returnUrl)}`) } diff --git a/web/app/page.spec.tsx b/web/app/page.spec.tsx new file mode 100644 index 0000000000..c97610d6b9 --- /dev/null +++ b/web/app/page.spec.tsx @@ -0,0 +1,71 @@ +import { redirect } from 'next/navigation' +import Home from './page' + +vi.mock('next/navigation', () => ({ + redirect: vi.fn(), +})) + +describe('Home page redirect', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('should redirect to /apps when search params are empty', async () => { + // Arrange + const props = { + searchParams: Promise.resolve({}), + } + + // Act + await Home(props) + + // Assert + expect(redirect).toHaveBeenCalledWith('/apps') + }) + + it('should preserve single query param when redirecting to /apps', async () => { + // Arrange + const props = { + searchParams: Promise.resolve({ + oauth_redirect_url: 'https://example.com/callback', + }), + } + + // Act + await Home(props) + + // Assert + expect(redirect).toHaveBeenCalledWith('/apps?oauth_redirect_url=https%3A%2F%2Fexample.com%2Fcallback') + }) + + it('should preserve repeated query params when redirecting to /apps', async () => { + // Arrange + const props = { + searchParams: Promise.resolve({ + scope: ['read:name', 'read:email'], + }), + } + + // Act + await Home(props) + + // Assert + expect(redirect).toHaveBeenCalledWith('/apps?scope=read%3Aname&scope=read%3Aemail') + }) + + it('should ignore undefined query values when building redirect url', async () => { + // Arrange + const props = { + searchParams: Promise.resolve({ + client_id: 'abc', + state: undefined, + }), + } + + // Act + await Home(props) + + // Assert + expect(redirect).toHaveBeenCalledWith('/apps?client_id=abc') + }) +}) diff --git a/web/app/signin/utils/post-login-redirect.spec.ts b/web/app/signin/utils/post-login-redirect.spec.ts new file mode 100644 index 0000000000..f6839319d4 --- /dev/null +++ b/web/app/signin/utils/post-login-redirect.spec.ts @@ -0,0 +1,149 @@ +import type { ReadonlyURLSearchParams } from 'next/navigation' +import { OAUTH_AUTHORIZE_PENDING_KEY, REDIRECT_URL_KEY } from '@/app/account/oauth/authorize/constants' +import { storage } from '@/utils/storage' +import { resolvePostLoginRedirect } from './post-login-redirect' + +const FIXED_DATE = new Date('2026-02-10T12:00:00.000Z') + +const createSearchParams = (params: Record) => { + return new URLSearchParams(params) as unknown as ReadonlyURLSearchParams +} + +const setPendingRedirect = (value: unknown) => { + storage.set(OAUTH_AUTHORIZE_PENDING_KEY, value as never) +} + +describe('resolvePostLoginRedirect', () => { + let consoleErrorSpy: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + storage.resetCache() + localStorage.clear() + vi.useFakeTimers() + vi.setSystemTime(FIXED_DATE) + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + vi.useRealTimers() + consoleErrorSpy.mockRestore() + }) + + describe('redirect_url priority', () => { + it('should return decoded redirect_url when query param exists', () => { + // Arrange + setPendingRedirect({ + value: '/stale-pending', + expiry: Math.floor(Date.now() / 1000) + 60, + }) + const redirectUrl = 'https://example.com/account/oauth/authorize?client_id=abc' + const searchParams = createSearchParams({ + [REDIRECT_URL_KEY]: encodeURIComponent(redirectUrl), + }) + + // Act + const result = resolvePostLoginRedirect(searchParams) + + // Assert + expect(result).toBe(redirectUrl) + expect(storage.get(OAUTH_AUTHORIZE_PENDING_KEY)).toBeNull() + }) + + it('should return original redirect_url when decoding fails', () => { + // Arrange + setPendingRedirect({ + value: '/stale-pending', + expiry: Math.floor(Date.now() / 1000) + 60, + }) + const malformedRedirectUrl = '%E0%A4%A' + const searchParams = createSearchParams({ + [REDIRECT_URL_KEY]: malformedRedirectUrl, + }) + + // Act + const result = resolvePostLoginRedirect(searchParams) + + // Assert + expect(result).toBe(malformedRedirectUrl) + expect(storage.get(OAUTH_AUTHORIZE_PENDING_KEY)).toBeNull() + expect(consoleErrorSpy).toHaveBeenCalledTimes(1) + }) + }) + + describe('pending redirect fallback', () => { + it('should return pending redirect when redirect_url is absent and pending is valid', () => { + // Arrange + const pendingRedirect = 'https://skills.bash-is-all-you-need.dify.dev/account/oauth/authorize?client_id=dcfcd6a4-5799-405a-a6d7-04261b24dd02&redirect_uri=https%3A%2F%2Fcreators.dify.dev%2Fapi%2Fv1%2Foauth%2Fcallback%2Fdify&response_type=code' + setPendingRedirect({ + value: pendingRedirect, + expiry: Math.floor(Date.now() / 1000) + 60, + }) + + // Act + const result = resolvePostLoginRedirect(createSearchParams({})) + + // Assert + expect(result).toBe(pendingRedirect) + expect(storage.get(OAUTH_AUTHORIZE_PENDING_KEY)).toBeNull() + }) + + it('should consume pending redirect only once', () => { + // Arrange + const pendingRedirect = '/account/oauth/authorize?client_id=one-time' + setPendingRedirect({ + value: pendingRedirect, + expiry: Math.floor(Date.now() / 1000) + 60, + }) + + // Act + const firstResult = resolvePostLoginRedirect(createSearchParams({})) + const secondResult = resolvePostLoginRedirect(createSearchParams({})) + + // Assert + expect(firstResult).toBe(pendingRedirect) + expect(secondResult).toBeNull() + }) + + it('should return null when pending redirect is expired', () => { + // Arrange + setPendingRedirect({ + value: '/account/oauth/authorize?client_id=expired', + expiry: Math.floor(Date.now() / 1000) - 1, + }) + + // Act + const result = resolvePostLoginRedirect(createSearchParams({})) + + // Assert + expect(result).toBeNull() + expect(storage.get(OAUTH_AUTHORIZE_PENDING_KEY)).toBeNull() + }) + + it('should return null when pending redirect payload is invalid', () => { + // Arrange + setPendingRedirect({ + value: '/account/oauth/authorize?client_id=invalid', + }) + + // Act + const result = resolvePostLoginRedirect(createSearchParams({})) + + // Assert + expect(result).toBeNull() + }) + }) + + describe('empty state', () => { + it('should return null when no redirect_url and no pending redirect exist', () => { + // Arrange + const searchParams = createSearchParams({}) + + // Act + const result = resolvePostLoginRedirect(searchParams) + + // Assert + expect(result).toBeNull() + }) + }) +}) diff --git a/web/app/signin/utils/post-login-redirect.ts b/web/app/signin/utils/post-login-redirect.ts index c106a629a7..8dd36d2524 100644 --- a/web/app/signin/utils/post-login-redirect.ts +++ b/web/app/signin/utils/post-login-redirect.ts @@ -8,28 +8,12 @@ type OAuthPendingRedirect = { expiry?: number } -function getLegacyOAuthPendingRedirect(): OAuthPendingRedirect | null { - try { - const itemStr = localStorage.getItem(OAUTH_AUTHORIZE_PENDING_KEY) - return itemStr ? JSON.parse(itemStr) : null - } - catch { - return null - } -} - function removeOAuthPendingRedirect() { storage.remove(OAUTH_AUTHORIZE_PENDING_KEY) - try { - localStorage.removeItem(OAUTH_AUTHORIZE_PENDING_KEY) - } - catch { - // ignore legacy key cleanup failures - } } function getOAuthPendingRedirect(): string | null { - const item = storage.get(OAUTH_AUTHORIZE_PENDING_KEY) ?? getLegacyOAuthPendingRedirect() + const item = storage.get(OAUTH_AUTHORIZE_PENDING_KEY) if (!item) return null