fix(auth): preserve redirect URL across auth flows (#38905)

Co-authored-by: CodingOnStar <hanxujiang@dify.com>
This commit is contained in:
Coding On Star
2026-07-14 15:47:34 +08:00
committed by GitHub
parent 35ace0ea18
commit 2c4d8ef098
13 changed files with 435 additions and 19 deletions

View File

@ -0,0 +1,109 @@
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
import { useRouter, useSearchParams } from '@/next/navigation'
import { changePasswordWithToken } from '@/service/common'
import ChangePasswordForm from '../page'
const countdownState = vi.hoisted(() => ({
onEnd: undefined as (() => void) | undefined,
}))
vi.mock('ahooks', () => ({
useCountDown: ({ leftTime, onEnd }: { leftTime?: number; onEnd?: () => void }) => {
countdownState.onEnd = onEnd
return [leftTime ?? 0]
},
}))
vi.mock('@/next/navigation', () => ({
useRouter: vi.fn(),
useSearchParams: vi.fn(),
}))
vi.mock('@/service/common', () => ({
changePasswordWithToken: vi.fn(),
}))
const mockReplace = vi.fn()
const mockUseRouter = vi.mocked(useRouter)
const mockUseSearchParams = vi.mocked(useSearchParams)
const mockChangePasswordWithToken = vi.mocked(changePasswordWithToken)
const redirectUrl = '/apps?template-id=template-1&utm_source=dify_blog'
const encodedSigninUrl =
'/signin?redirect_url=%2Fapps%3Ftemplate-id%3Dtemplate-1%26utm_source%3Ddify_blog'
const setSearchParams = (params: Record<string, string>) => {
mockUseSearchParams.mockReturnValue(
new URLSearchParams(params) as unknown as ReturnType<typeof useSearchParams>,
)
}
const completePasswordChange = async () => {
render(<ChangePasswordForm />)
fireEvent.change(screen.getByLabelText('common.account.newPassword'), {
target: { value: 'ValidPass123!' },
})
fireEvent.change(screen.getByLabelText('common.account.confirmPassword'), {
target: { value: 'ValidPass123!' },
})
fireEvent.click(screen.getByRole('button', { name: 'login.changePasswordBtn' }))
await waitFor(() => {
expect(screen.getByRole('button', { name: /login\.passwordChanged/ })).toBeInTheDocument()
})
}
describe('Reset Password Set Password Page', () => {
beforeEach(() => {
vi.clearAllMocks()
countdownState.onEnd = undefined
mockUseRouter.mockReturnValue({ replace: mockReplace } as unknown as ReturnType<
typeof useRouter
>)
mockChangePasswordWithToken.mockResolvedValue({ result: 'success' })
setSearchParams({ token: 'reset-token' })
})
describe('Post-reset navigation', () => {
it('should preserve redirect_url when the user returns to sign in manually', async () => {
setSearchParams({ token: 'reset-token', redirect_url: redirectUrl })
await completePasswordChange()
fireEvent.click(screen.getByRole('button', { name: /login\.passwordChanged/ }))
expect(mockReplace).toHaveBeenCalledWith(encodedSigninUrl)
})
it('should preserve redirect_url when the countdown returns to sign in automatically', async () => {
setSearchParams({ token: 'reset-token', redirect_url: redirectUrl })
await completePasswordChange()
expect(countdownState.onEnd).toBeTypeOf('function')
act(() => countdownState.onEnd?.())
expect(mockReplace).toHaveBeenCalledWith(encodedSigninUrl)
})
it('should preserve the activation redirect when an invite token is present', async () => {
setSearchParams({
token: 'reset-token',
invite_token: 'invite-token',
redirect_url: redirectUrl,
})
await completePasswordChange()
fireEvent.click(screen.getByRole('button', { name: /login\.passwordChanged/ }))
expect(mockReplace).toHaveBeenCalledWith('/activate?token=invite-token')
})
it('should return to plain sign in when no redirect target is present', async () => {
await completePasswordChange()
fireEvent.click(screen.getByRole('button', { name: /login\.passwordChanged/ }))
expect(mockReplace).toHaveBeenCalledWith('/signin')
})
})
})

View File

@ -33,6 +33,14 @@ const ChangePasswordForm = () => {
params.set('token', searchParams.get('invite_token') as string)
return `/activate?${params.toString()}`
}
const redirectUrl = searchParams.get('redirect_url')
if (redirectUrl) {
const params = new URLSearchParams()
params.set('redirect_url', redirectUrl)
return `/signin?${params.toString()}`
}
return '/signin'
}

View File

@ -1,5 +1,5 @@
import { useQuery, useSuspenseQuery } from '@tanstack/react-query'
import { render, waitFor } from '@testing-library/react'
import { QueryClient, QueryClientProvider, useQuery, useSuspenseQuery } from '@tanstack/react-query'
import { render, screen, waitFor } from '@testing-library/react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useRouter, useSearchParams } from '@/next/navigation'
import NormalForm from '../normal-form'
@ -147,4 +147,41 @@ describe('NormalForm', () => {
})
})
})
describe('Registration Navigation', () => {
it('should preserve the current query when navigating to sign up', () => {
mockUseSearchParams.mockReturnValue(
new URLSearchParams('redirect_url=%2Fapps%3Ftag%3Dworkflow&source=pricing'),
)
mockUseQuery.mockReturnValue(nonInviteQueryResult as unknown as ReturnType<typeof useQuery>)
mockUseSuspenseQuery.mockReturnValue({
data: {
enable_social_oauth_login: false,
sso_enforced_for_signin: false,
enable_email_code_login: false,
enable_email_password_login: true,
is_email_setup: true,
is_allow_register: true,
license: {
status: 'none',
},
branding: {
enabled: true,
},
},
} as unknown as ReturnType<typeof useSuspenseQuery>)
const queryClient = new QueryClient()
render(
<QueryClientProvider client={queryClient}>
<NormalForm />
</QueryClientProvider>,
)
expect(screen.getByRole('link', { name: 'login.signup.signUp' })).toHaveAttribute(
'href',
'/signup?redirect_url=%2Fapps%3Ftag%3Dworkflow&source=pricing',
)
})
})
})

View File

@ -0,0 +1,62 @@
import type { MockedFunction } from 'vitest'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { render, screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useRouter, useSearchParams } from '@/next/navigation'
import { useOneMoreStep } from '@/service/use-common'
import OneMoreStep from '../one-more-step'
vi.mock('@/next/navigation', () => ({
useRouter: vi.fn(),
useSearchParams: vi.fn(),
}))
vi.mock('@/service/use-common', () => ({
useOneMoreStep: vi.fn(),
}))
const mockReplace = vi.fn()
const mockSubmitOneMoreStep = vi.fn()
const mockUseRouter = useRouter as unknown as MockedFunction<typeof useRouter>
const mockUseSearchParams = useSearchParams as unknown as MockedFunction<typeof useSearchParams>
const mockUseOneMoreStep = useOneMoreStep as unknown as MockedFunction<typeof useOneMoreStep>
describe('OneMoreStep', () => {
beforeEach(() => {
vi.clearAllMocks()
mockUseRouter.mockReturnValue({ replace: mockReplace } as unknown as ReturnType<
typeof useRouter
>)
mockUseSearchParams.mockReturnValue(
new URLSearchParams(
'invitation_code=invite-code&redirect_url=%2Fapps%3Ftag%3Dworkflow',
) as unknown as ReturnType<typeof useSearchParams>,
)
mockUseOneMoreStep.mockReturnValue({
mutateAsync: mockSubmitOneMoreStep,
isPending: false,
} as unknown as ReturnType<typeof useOneMoreStep>)
mockSubmitOneMoreStep.mockResolvedValue({ result: 'success' })
})
// Successful account initialization returns users to their original console destination.
describe('Post-registration redirect', () => {
it('should return to the requested console page when account initialization succeeds', async () => {
const user = userEvent.setup()
const queryClient = new QueryClient()
render(
<QueryClientProvider client={queryClient}>
<OneMoreStep />
</QueryClientProvider>,
)
await user.click(screen.getByRole('button', { name: 'login.go' }))
await waitFor(() => {
expect(mockReplace).toHaveBeenCalledWith('/apps?tag=workflow')
})
})
})
})

View File

@ -27,6 +27,8 @@ function NormalForm() {
const { t } = useTranslation()
const router = useRouter()
const searchParams = useSearchParams()
const queryString = searchParams.toString()
const signupHref = queryString ? `/signup?${queryString}` : '/signup'
// Login probe: 401 stays as `error` (legitimate "not logged in" state on /signin),
// other errors throw to error.tsx. jumpTo same-pathname guard in service/base.ts
// prevents the redirect loop on 401.
@ -270,7 +272,7 @@ function NormalForm() {
{systemFeatures.is_allow_register && authType === 'password' && (
<div className="mb-3 text-[13px] leading-4 font-medium text-text-secondary">
<span>{t(($) => $['signup.noAccount'], { ns: 'login' })}</span>
<Link className="text-text-accent" href="/signup">
<Link className="text-text-accent" href={signupHref}>
{t(($) => $['signup.signUp'], { ns: 'login' })}
</Link>
</div>

View File

@ -14,13 +14,16 @@ import { toast } from '@langgenius/dify-ui/toast'
import { useQueryClient } from '@tanstack/react-query'
import { useReducer } from 'react'
import { useTranslation } from 'react-i18next'
import { resolvePostLoginRedirect } from '@/app/signin/utils/post-login-redirect'
import { LICENSE_LINK } from '@/constants/link'
import { languages } from '@/i18n-config/language'
import Link from '@/next/link'
import { useRouter, useSearchParams } from '@/next/navigation'
import { consoleQuery } from '@/service/client'
import { useOneMoreStep } from '@/service/use-common'
import { replaceLoginRedirect } from '@/utils/login-redirect.client'
import { timezones } from '@/utils/timezone'
import { basePath } from '@/utils/var'
import Input from '../components/base/input'
type IState = {
@ -108,7 +111,7 @@ const OneMoreStep = () => {
timezone: state.timezone,
})
await queryClient.resetQueries({ queryKey: consoleQuery.account.profile.get.key() })
router.replace('/')
replaceLoginRedirect(resolvePostLoginRedirect(searchParams), router.replace, basePath)
} catch (error: unknown) {
if (hasStatus(error) && error.status === 400)
toast.error(t(($) => $.invalidInvitationCode, { ns: 'login' }))

View File

@ -3,6 +3,7 @@ import { fireEvent, screen, waitFor } from '@testing-library/react'
import * as React from 'react'
import { renderWithSystemFeatures } from '@/__tests__/utils/mock-system-features'
import { useLocale } from '@/context/i18n'
import { useSearchParams } from '@/next/navigation'
import { useSendMail } from '@/service/use-common'
import Form from './input-mail'
@ -33,6 +34,10 @@ vi.mock('@/context/i18n', () => ({
useLocale: vi.fn(),
}))
vi.mock('@/next/navigation', () => ({
useSearchParams: vi.fn(),
}))
vi.mock('@/service/use-common', () => ({
useSendMail: vi.fn(),
}))
@ -40,6 +45,7 @@ vi.mock('@/service/use-common', () => ({
type UseSendMailResult = ReturnType<typeof useSendMail>
const mockUseLocale = useLocale as unknown as MockedFunction<typeof useLocale>
const mockUseSearchParams = useSearchParams as unknown as MockedFunction<typeof useSearchParams>
const mockUseSendMail = useSendMail as unknown as MockedFunction<typeof useSendMail>
const renderForm = ({
@ -62,6 +68,9 @@ const renderForm = ({
describe('InputMail Form', () => {
beforeEach(() => {
vi.clearAllMocks()
mockUseSearchParams.mockReturnValue(
new URLSearchParams() as unknown as ReturnType<typeof useSearchParams>,
)
mockSubmitMail.mockResolvedValue({ result: 'success', data: 'token' })
})
@ -114,6 +123,24 @@ describe('InputMail Form', () => {
})
})
// Navigation between registration and login keeps the original destination.
describe('Registration Navigation', () => {
it('should preserve the current query when navigating to sign in', () => {
mockUseSearchParams.mockReturnValue(
new URLSearchParams(
'redirect_url=%2Fapps%3Ftag%3Dworkflow&source=pricing',
) as unknown as ReturnType<typeof useSearchParams>,
)
renderForm()
expect(screen.getByRole('link', { name: 'login.signup.signIn' })).toHaveAttribute(
'href',
'/signin?redirect_url=%2Fapps%3Ftag%3Dworkflow&source=pricing',
)
})
})
// Validation and failure paths.
describe('Edge Cases', () => {
it('should block submission when email is invalid', () => {

View File

@ -11,6 +11,7 @@ import { emailRegex } from '@/config'
import { useLocale } from '@/context/i18n'
import { systemFeaturesQueryOptions } from '@/features/system-features/client'
import Link from '@/next/link'
import { useSearchParams } from '@/next/navigation'
import { useSendMail } from '@/service/use-common'
type Props = {
@ -20,6 +21,9 @@ export default function Form({ onSuccess }: Props) {
const { t } = useTranslation()
const [email, setEmail] = useState('')
const locale = useLocale()
const searchParams = useSearchParams()
const queryString = searchParams.toString()
const signinHref = queryString ? `/signin?${queryString}` : '/signin'
const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions())
const { mutateAsync: submitMail, isPending } = useSendMail()
@ -78,7 +82,7 @@ export default function Form({ onSuccess }: Props) {
<div className="text-[13px] leading-4 font-medium text-text-secondary">
<span>{t(($) => $['signup.haveAccount'], { ns: 'login' })}</span>
<Link className="text-text-accent" href="/signin">
<Link className="text-text-accent" href={signinHref}>
{t(($) => $['signup.signIn'], { ns: 'login' })}
</Link>
</div>

View File

@ -145,6 +145,22 @@ describe('Signup Set Password Page', () => {
expect(mockReplace).toHaveBeenCalledWith('/')
})
it('should return to the requested console page when registration succeeds', async () => {
mockUseSearchParams.mockReturnValue(
new URLSearchParams(
'token=register-token&redirect_url=%2Fapps%3Ftag%3Dworkflow',
) as unknown as ReturnType<typeof useSearchParams>,
)
mockRegister.mockResolvedValue({ result: 'success', data: {} })
renderWithQueryClient(<ChangePasswordForm />)
fillAndSubmit()
await waitFor(() => {
expect(mockReplace).toHaveBeenCalledWith('/apps?tag=workflow')
})
})
it('should remember the utm event with slug and clear the utm cookie when a utm_info cookie is present', async () => {
Cookies.set('utm_info', JSON.stringify({ utm_source: 'community', slug: 'partner-launch' }))
mockRegister.mockResolvedValue({ result: 'success', data: {} })

View File

@ -9,6 +9,7 @@ import { useCallback, useState } from 'react'
import { useTranslation } from 'react-i18next'
import { rememberRegistrationSuccess } from '@/app/components/base/amplitude/registration-tracking'
import Input from '@/app/components/base/input'
import { resolvePostLoginRedirect } from '@/app/signin/utils/post-login-redirect'
import { validPassword } from '@/config'
import { useLocale } from '@/context/i18n'
import { useRouter, useSearchParams } from '@/next/navigation'
@ -16,7 +17,9 @@ import { consoleQuery } from '@/service/client'
import { useMailRegister } from '@/service/use-common'
import { rememberCreateAppExternalAttribution } from '@/utils/create-app-tracking'
import { sendGAEvent } from '@/utils/gtag'
import { replaceLoginRedirect } from '@/utils/login-redirect.client'
import { getBrowserTimezone } from '@/utils/timezone'
import { basePath } from '@/utils/var'
const parseUtmInfo = () => {
const utmInfoStr = Cookies.get('utm_info')
@ -88,12 +91,23 @@ const ChangePasswordForm = () => {
toast.success(t(($) => $['api.actionSuccess'], { ns: 'common' }))
await queryClient.resetQueries({ queryKey: consoleQuery.account.profile.get.key() })
router.replace('/')
replaceLoginRedirect(resolvePostLoginRedirect(searchParams), router.replace, basePath)
}
} catch (error) {
console.error(error)
}
}, [password, token, valid, confirmPassword, register, locale, queryClient, router, t])
}, [
password,
token,
valid,
confirmPassword,
register,
locale,
queryClient,
router,
searchParams,
t,
])
return (
<div

View File

@ -1,4 +1,4 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const createUnauthorizedResponse = () =>
new Response(
@ -49,8 +49,75 @@ async function loadServerRequest() {
}
}
type ClientRequestOptions = {
response: Response
refreshError?: Error
}
async function loadClientRequest({ response, refreshError }: ClientRequestOptions) {
vi.resetModules()
const mockBaseFetch = vi.fn(async () => {
throw response
})
const mockRefreshAccessTokenOrReLogin = refreshError
? vi.fn().mockRejectedValue(refreshError)
: vi.fn()
vi.doMock('@/utils/client', () => ({
isClient: true,
isServer: false,
}))
vi.doMock('@/utils/var', () => ({
basePath: '/app',
}))
vi.doMock('../fetch', () => ({
base: mockBaseFetch,
ContentType: {
audio: 'audio/mpeg',
download: 'application/octet-stream',
downloadZip: 'application/zip',
json: 'application/json',
},
getBaseOptions: vi.fn(() => ({})),
}))
vi.doMock('../refresh-token', () => ({
refreshAccessTokenOrReLogin: mockRefreshAccessTokenOrReLogin,
}))
const { request } = await import('../base')
return {
request,
mockRefreshAccessTokenOrReLogin,
}
}
describe('request 401 handling', () => {
const originalLocation = globalThis.location
beforeEach(() => {
vi.clearAllMocks()
Object.defineProperty(globalThis, 'location', {
value: {
origin: 'https://example.com',
pathname: '/app/apps',
search: '?category=agent',
hash: '#recent',
href: 'https://example.com/app/apps?category=agent#recent',
reload: vi.fn(),
},
writable: true,
configurable: true,
})
})
afterEach(() => {
Object.defineProperty(globalThis, 'location', {
value: originalLocation,
writable: true,
configurable: true,
})
vi.resetModules()
vi.restoreAllMocks()
})
@ -62,4 +129,31 @@ describe('request 401 handling', () => {
expect(mockRefreshAccessTokenOrReLogin).not.toHaveBeenCalled()
})
it('should preserve the current URL when a 401 response cannot be parsed', async () => {
const response = new Response('not-json', { status: 401 })
const { request, mockRefreshAccessTokenOrReLogin } = await loadClientRequest({ response })
await expect(request('/account/profile')).rejects.toBe(response)
expect(globalThis.location.href).toBe(
`https://example.com/app/signin?redirect_url=${encodeURIComponent('/app/apps?category=agent#recent')}`,
)
expect(mockRefreshAccessTokenOrReLogin).not.toHaveBeenCalled()
})
it('should preserve the current URL when token refresh fails', async () => {
const response = createUnauthorizedResponse()
const { request, mockRefreshAccessTokenOrReLogin } = await loadClientRequest({
response,
refreshError: new Error('refresh failed'),
})
await expect(request('/account/profile')).rejects.toBe(response)
expect(mockRefreshAccessTokenOrReLogin).toHaveBeenCalledOnce()
expect(globalThis.location.href).toBe(
`https://example.com/app/signin?redirect_url=${encodeURIComponent('/app/apps?category=agent#recent')}`,
)
})
})

View File

@ -18,8 +18,10 @@ describe('buildSigninUrlWithRedirect', () => {
Object.defineProperty(globalThis, 'location', {
value: {
origin: 'https://example.com',
pathname: '/apps',
href: 'https://example.com/apps',
pathname: '/app/apps',
search: '?category=agent',
hash: '#recent',
href: 'https://example.com/app/apps?category=agent#recent',
},
writable: true,
configurable: true,
@ -34,9 +36,11 @@ describe('buildSigninUrlWithRedirect', () => {
})
})
it('should return plain signin URL for non-OAuth pages', () => {
it('should preserve the current internal URL for Console pages', () => {
const url = buildSigninUrlWithRedirect()
expect(url).toBe('https://example.com/app/signin')
expect(url).toBe(
`https://example.com/app/signin?redirect_url=${encodeURIComponent('/app/apps?category=agent#recent')}`,
)
})
it('should append redirect_url for OAuth authorize pages', () => {
@ -45,6 +49,8 @@ describe('buildSigninUrlWithRedirect', () => {
value: {
origin: 'https://example.com',
pathname: '/account/oauth/authorize',
search: '?client_id=abc&state=xyz',
hash: '',
href: oauthHref,
},
writable: true,
@ -55,11 +61,13 @@ describe('buildSigninUrlWithRedirect', () => {
expect(url).toBe(`https://example.com/app/signin?redirect_url=${encodeURIComponent(oauthHref)}`)
})
it('should not include redirect_url for other paths containing partial match', () => {
it('should treat other paths containing a partial OAuth match as Console pages', () => {
Object.defineProperty(globalThis, 'location', {
value: {
origin: 'https://example.com',
pathname: '/settings/oauth',
search: '',
hash: '',
href: 'https://example.com/settings/oauth',
},
writable: true,
@ -67,8 +75,29 @@ describe('buildSigninUrlWithRedirect', () => {
})
const url = buildSigninUrlWithRedirect()
expect(url).toBe('https://example.com/app/signin')
expect(url).toBe(
`https://example.com/app/signin?redirect_url=${encodeURIComponent('/settings/oauth')}`,
)
})
it.each(['/app/signin', '/app/signin/'])(
'should not create a self-referential redirect for the signin page: %s',
(pathname) => {
Object.defineProperty(globalThis, 'location', {
value: {
origin: 'https://example.com',
pathname,
search: '?redirect_url=%2Fapp%2Fapps',
hash: '',
href: `https://example.com${pathname}?redirect_url=%2Fapp%2Fapps`,
},
writable: true,
configurable: true,
})
expect(buildSigninUrlWithRedirect()).toBe('https://example.com/app/signin')
},
)
})
describe('buildWebAppSigninUrlWithRedirect', () => {

View File

@ -41,6 +41,7 @@ import {
} from '@/config'
import { asyncRunSafe } from '@/utils'
import { isClient } from '@/utils/client'
import { resolveLoginRedirectTarget } from '@/utils/login-redirect'
import { basePath } from '@/utils/var'
import { base, ContentType, getBaseOptions } from './fetch'
import { refreshAccessTokenOrReLogin } from './refresh-token'
@ -162,17 +163,28 @@ function jumpTo(url: string) {
}
const OAUTH_AUTHORIZE_PATH = '/account/oauth/authorize'
const SIGNIN_PATH = '/signin'
export const buildSigninUrlWithRedirect = (): string => {
const loginUrl = `${isClient ? window.location.origin : ''}${basePath}/signin`
if (!isClient) return loginUrl
// Only preserve redirect URL for OAuth authorize pages
if (isClient && window.location.pathname.includes(OAUTH_AUTHORIZE_PATH)) {
const signinPath = `${basePath}${SIGNIN_PATH}`
if (window.location.pathname === signinPath || window.location.pathname === `${signinPath}/`)
return loginUrl
if (window.location.pathname.includes(OAUTH_AUTHORIZE_PATH)) {
const currentUrl = window.location.href
return `${loginUrl}?redirect_url=${encodeURIComponent(currentUrl)}`
}
return loginUrl
const currentTarget = resolveLoginRedirectTarget(
`${window.location.pathname}${window.location.search}${window.location.hash}`,
{ allowSameOriginAbsolute: false },
)
if (!currentTarget || currentTarget.kind !== 'internal') return loginUrl
return `${loginUrl}?redirect_url=${encodeURIComponent(currentTarget.href)}`
}
function unicodeToChar(text: string) {
@ -941,9 +953,8 @@ export const request = async <T>(url: string, options = {}, otherOptions?: IOthe
if (!isClient) return Promise.reject(err)
const [parseErr, errRespData] = await asyncRunSafe<ResponseError>(errResp.json())
const loginUrl = `${window.location.origin}${basePath}/signin`
if (parseErr) {
window.location.href = loginUrl
window.location.href = buildSigninUrlWithRedirect()
return Promise.reject(err)
}
if (/\/login/.test(url)) return Promise.reject(errRespData)