mirror of
https://github.com/langgenius/dify.git
synced 2026-05-06 02:18:08 +08:00
refactor(web): preserve all OAuth query params and remove legacy storage fallback
- Use searchParams.toString() to forward all query params instead of manually encoding only client_id and redirect_uri - Remove legacy localStorage fallback since storage utility is now the sole persistence layer - Add tests for OAuth authorize page, home page, and post-login redirect
This commit is contained in:
127
web/app/account/oauth/authorize/page.spec.tsx
Normal file
127
web/app/account/oauth/authorize/page.spec.tsx
Normal file
@ -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(<OAuthAuthorize />)
|
||||
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(<OAuthAuthorize />)
|
||||
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)
|
||||
})
|
||||
})
|
||||
@ -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)}`)
|
||||
}
|
||||
|
||||
71
web/app/page.spec.tsx
Normal file
71
web/app/page.spec.tsx
Normal file
@ -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')
|
||||
})
|
||||
})
|
||||
149
web/app/signin/utils/post-login-redirect.spec.ts
Normal file
149
web/app/signin/utils/post-login-redirect.spec.ts
Normal file
@ -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<string, string>) => {
|
||||
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<typeof vi.spyOn>
|
||||
|
||||
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()
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -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<OAuthPendingRedirect>(OAUTH_AUTHORIZE_PENDING_KEY) ?? getLegacyOAuthPendingRedirect()
|
||||
const item = storage.get<OAuthPendingRedirect>(OAUTH_AUTHORIZE_PENDING_KEY)
|
||||
if (!item)
|
||||
return null
|
||||
|
||||
|
||||
Reference in New Issue
Block a user