fix(web): isolate OIDC discovery when saving OAuth settings

Run discovery only when its URL changes or OIDC is enabled. Show URL validation errors beside the field and keep save actions disabled throughout submission. Correct Well-Known terminology in translations and cover provider-independent saves, validation, discovery failures, and loading state. Validation from web/: bun run test src/features/system-settings/auth/__tests__/oauth-settings.test.tsx src/features/system-settings/auth/__tests__/passkey-settings.test.tsx (30 passed); bun run typecheck; bunx oxlint -c .oxlintrc.json src/features/system-settings/auth/oauth-section.tsx src/features/system-settings/auth/__tests__/oauth-settings.test.tsx; bunx oxfmt --check on both files; bun run build; bun run i18n:sync. In-app browser checked GitHub save and inline OIDC error using an isolated fixture with mocked API responses. Security scope: settings form only. Consulted OWASP ASVS 5.0.0 section 8.3.1 and the Authentication, Session Management, and OAuth2 Cheat Sheets. Server authorization and authentication protocol flows are unchanged; this is not a full authentication compliance assessment.
2026-11-09 18:07:04 +08:00 · 2026-09-20 17:51:51 +08:00
parent 9c3d3aeb3c
commit 33320407f4
8 changed files with 360 additions and 11 deletions
@@ -0,0 +1,339 @@
+/*
+Copyright (C) 2023-2026 QuantumNous
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as
+published by the Free Software Foundation, either version 3 of the
+License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+For commercial licensing, please contact [email protected]
+*/
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
+import {
+  createMemoryHistory,
+  createRootRoute,
+  createRouter,
+  RouterProvider,
+} from '@tanstack/react-router'
+import { render, screen, waitFor } from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
+import axios from 'axios'
+import { createInstance, type i18n } from 'i18next'
+import { useState, type ComponentProps } from 'react'
+import { I18nextProvider } from 'react-i18next'
+import { toast } from 'sonner'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import zhTW from '@/i18n/locales/zh-TW.json'
+import zh from '@/i18n/locales/zh.json'
+import { api } from '@/lib/api'
+
+import { SettingsPageProvider } from '../../components/settings-page-context'
+import { OAuthSection } from '../oauth-section'
+
+const defaults: ComponentProps<typeof OAuthSection>['defaultValues'] = {
+  GitHubOAuthEnabled: false,
+  GitHubClientId: '',
+  GitHubClientSecret: '',
+  'discord.enabled': false,
+  'discord.client_id': '',
+  'discord.client_secret': '',
+  'oidc.enabled': false,
+  'oidc.display_name': '',
+  'oidc.client_id': '',
+  'oidc.client_secret': '',
+  'oidc.well_known': '',
+  'oidc.authorization_endpoint': '',
+  'oidc.token_endpoint': '',
+  'oidc.user_info_endpoint': '',
+  TelegramOAuthEnabled: false,
+  'telegram.client_id': '',
+  'telegram.client_secret': '',
+  LinuxDOOAuthEnabled: false,
+  LinuxDOClientId: '',
+  LinuxDOClientSecret: '',
+  LinuxDOMinimumTrustLevel: '',
+  WeChatAuthEnabled: false,
+  WeChatServerAddress: '',
+  WeChatServerToken: '',
+  WeChatAccountQRCodeImageURL: '',
+}
+
+let testI18n: i18n
+
+function Fixture(props: { values?: Partial<typeof defaults> }) {
+  const [container, setContainer] = useState<HTMLDivElement | null>(null)
+  const [client] = useState(
+    () => new QueryClient({ defaultOptions: { mutations: { retry: false } } })
+  )
+
+  return (
+    <I18nextProvider i18n={testI18n}>
+      <QueryClientProvider client={client}>
+        <div ref={setContainer} />
+        <SettingsPageProvider actionsContainer={container}>
+          <OAuthSection
+            defaultValues={{ ...defaults, ...props.values }}
+            serverAddress='https://gateway.example.com'
+          />
+        </SettingsPageProvider>
+      </QueryClientProvider>
+    </I18nextProvider>
+  )
+}
+
+async function renderSettings(values?: Partial<typeof defaults>) {
+  const route = createRootRoute({
+    component: () => <Fixture values={values} />,
+  })
+  const router = createRouter({
+    routeTree: route,
+    history: createMemoryHistory({ initialEntries: ['/'] }),
+  })
+  await router.load()
+  render(<RouterProvider router={router} />)
+}
+
+beforeEach(async () => {
+  testI18n = createInstance()
+  await testI18n.init({
+    lng: 'en',
+    fallbackLng: 'en',
+    resources: { en: { translation: {} }, zh, 'zh-TW': zhTW },
+    interpolation: { escapeValue: false },
+  })
+  vi.spyOn(api, 'put').mockResolvedValue({ data: { success: true } })
+})
+
+describe('OAuth settings saves', () => {
+  it.each([
+    [false, 'provider.example.com/discovery'],
+    [true, 'provider.example.com/discovery'],
+    [true, 'https://provider.example.com/discovery'],
+  ])(
+    'saves GitHub independently of unchanged OIDC settings (enabled: %s, URL: %s)',
+    async (enabled, url) => {
+      const discoveryClient = axios.create()
+      vi.spyOn(discoveryClient, 'get').mockRejectedValue(
+        new Error('OIDC unavailable')
+      )
+      const discovery = vi
+        .spyOn(axios, 'create')
+        .mockReturnValue(discoveryClient)
+      const errorToast = vi.spyOn(toast, 'error')
+      const user = userEvent.setup()
+      await renderSettings({ 'oidc.enabled': enabled, 'oidc.well_known': url })
+      await user.type(
+        screen.getByRole('textbox', { name: 'Client ID' }),
+        'github-client'
+      )
+      await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+      await waitFor(() =>
+        expect(api.put).toHaveBeenCalledWith('/api/option/', {
+          key: 'GitHubClientId',
+          value: 'github-client',
+        })
+      )
+      expect(api.put).toHaveBeenCalledTimes(1)
+      expect(discovery).not.toHaveBeenCalled()
+      expect(errorToast).not.toHaveBeenCalled()
+      expect(screen.getByRole('tab', { name: 'GitHub' })).toHaveAttribute(
+        'aria-selected',
+        'true'
+      )
+      expect(screen.getByRole('button', { name: 'Reset' })).toBeDisabled()
+    }
+  )
+
+  it('does not discover OIDC when saving an unchanged form', async () => {
+    const discovery = vi.spyOn(axios, 'create')
+    const infoToast = vi.spyOn(toast, 'info')
+    const user = userEvent.setup()
+    await renderSettings({
+      'oidc.well_known': 'provider.example.com/discovery',
+    })
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+    expect(infoToast).toHaveBeenCalledWith('No changes to save')
+    expect(discovery).not.toHaveBeenCalled()
+    expect(api.put).not.toHaveBeenCalled()
+  })
+
+  it('allows disabling OIDC even when its saved discovery URL is invalid', async () => {
+    const user = userEvent.setup()
+    await renderSettings({
+      'oidc.enabled': true,
+      'oidc.well_known': 'provider.example.com/discovery',
+    })
+    await user.click(screen.getByRole('tab', { name: 'OIDC' }))
+    await user.click(screen.getByRole('switch', { name: 'Enable OIDC' }))
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+    await waitFor(() =>
+      expect(api.put).toHaveBeenCalledWith('/api/option/', {
+        key: 'oidc.enabled',
+        value: false,
+      })
+    )
+    expect(api.put).toHaveBeenCalledTimes(1)
+  })
+
+  it('opens OIDC and marks an edited invalid URL even when saving from the GitHub tab', async () => {
+    const errorToast = vi.spyOn(toast, 'error')
+    const user = userEvent.setup()
+    await renderSettings()
+    await user.click(screen.getByRole('tab', { name: 'OIDC' }))
+    await user.type(
+      screen.getByRole('textbox', { name: 'Well-Known URL' }),
+      'provider.example.com/discovery'
+    )
+    await user.click(screen.getByRole('tab', { name: 'GitHub' }))
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+    const url = await screen.findByRole('textbox', { name: 'Well-Known URL' })
+    expect(url).toHaveAttribute('aria-invalid', 'true')
+    expect(url).toHaveAccessibleDescription(
+      expect.stringContaining(
+        'Well-Known URL must start with http:// or https://'
+      )
+    )
+    expect(api.put).not.toHaveBeenCalled()
+    expect(errorToast).not.toHaveBeenCalled()
+  })
+
+  it('validates the saved discovery URL before enabling OIDC', async () => {
+    const user = userEvent.setup()
+    await renderSettings({
+      'oidc.well_known': 'provider.example.com/discovery',
+    })
+    await user.click(screen.getByRole('tab', { name: 'OIDC' }))
+    await user.click(screen.getByRole('switch', { name: 'Enable OIDC' }))
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+    expect(
+      screen.getByRole('textbox', { name: 'Well-Known URL' })
+    ).toHaveAttribute('aria-invalid', 'true')
+    expect(api.put).not.toHaveBeenCalled()
+  })
+
+  it('keeps Save disabled during discovery and persists the discovered endpoints', async () => {
+    const client = axios.create()
+    const response = {
+      data: {
+        authorization_endpoint: 'https://provider.example.com/authorize',
+        token_endpoint: 'https://provider.example.com/token',
+        userinfo_endpoint: 'https://provider.example.com/userinfo',
+      },
+    }
+    let finishDiscovery!: (value: typeof response) => void
+    const get = vi.spyOn(client, 'get').mockImplementation(
+      () =>
+        new Promise((resolve) => {
+          finishDiscovery = resolve
+        })
+    )
+    vi.spyOn(axios, 'create').mockReturnValue(client)
+    const user = userEvent.setup()
+    await renderSettings()
+    await user.click(screen.getByRole('tab', { name: 'OIDC' }))
+    await user.type(
+      screen.getByRole('textbox', { name: 'Well-Known URL' }),
+      'https://provider.example.com/discovery'
+    )
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+    await waitFor(() =>
+      expect(get).toHaveBeenCalledWith('https://provider.example.com/discovery')
+    )
+    expect(screen.getByRole('button', { name: 'Saving...' })).toBeDisabled()
+    expect(screen.getByRole('button', { name: 'Reset' })).toBeDisabled()
+    expect(api.put).not.toHaveBeenCalled()
+    finishDiscovery(response)
+    await waitFor(() =>
+      expect(screen.getByRole('button', { name: 'Reset' })).toBeDisabled()
+    )
+    await waitFor(() => expect(api.put).toHaveBeenCalledTimes(4))
+    expect(api.put).toHaveBeenCalledWith('/api/option/', {
+      key: 'oidc.authorization_endpoint',
+      value: response.data.authorization_endpoint,
+    })
+    expect(api.put).toHaveBeenCalledWith('/api/option/', {
+      key: 'oidc.token_endpoint',
+      value: response.data.token_endpoint,
+    })
+    expect(api.put).toHaveBeenCalledWith('/api/option/', {
+      key: 'oidc.user_info_endpoint',
+      value: response.data.userinfo_endpoint,
+    })
+    await waitFor(() =>
+      expect(screen.getByRole('button', { name: 'Save Changes' })).toBeEnabled()
+    )
+  })
+
+  it.each(['zh', 'zh-TW'])(
+    'uses the Well-Known technical term in %s validation',
+    async (language) => {
+      await testI18n.changeLanguage(language)
+      const user = userEvent.setup()
+      await renderSettings()
+      await user.click(screen.getByRole('tab', { name: 'OIDC' }))
+      await user.type(
+        screen.getByRole('textbox', { name: 'Well-Known URL' }),
+        'invalid'
+      )
+      await user.click(
+        screen.getByRole('button', { name: testI18n.t('Save Changes') })
+      )
+
+      const url = screen.getByRole('textbox', { name: 'Well-Known URL' })
+      expect(url).toHaveAccessibleDescription(
+        expect.stringContaining('Well-Known URL')
+      )
+      expect(url).not.toHaveAccessibleDescription(
+        expect.stringContaining('知名')
+      )
+    }
+  )
+
+  it('keeps edits available for retry when OIDC discovery fails', async () => {
+    const client = axios.create()
+    const get = vi
+      .spyOn(client, 'get')
+      .mockRejectedValue(new Error('OIDC unavailable'))
+    vi.spyOn(axios, 'create').mockReturnValue(client)
+    const errorToast = vi.spyOn(toast, 'error')
+    const user = userEvent.setup()
+    await renderSettings()
+    await user.click(screen.getByRole('tab', { name: 'OIDC' }))
+    await user.type(
+      screen.getByRole('textbox', { name: 'Well-Known URL' }),
+      'https://provider.example.com/discovery'
+    )
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+
+    await waitFor(() => expect(errorToast).toHaveBeenCalledTimes(1))
+    expect(api.put).not.toHaveBeenCalled()
+    expect(screen.getByRole('button', { name: 'Save Changes' })).toBeEnabled()
+    expect(screen.getByRole('button', { name: 'Reset' })).toBeEnabled()
+    expect(screen.getByRole('textbox', { name: 'Well-Known URL' })).toHaveValue(
+      'https://provider.example.com/discovery'
+    )
+    get.mockResolvedValue({ data: {} })
+    await user.click(screen.getByRole('button', { name: 'Save Changes' }))
+    await waitFor(() =>
+      expect(api.put).toHaveBeenCalledWith('/api/option/', {
+        key: 'oidc.well_known',
+        value: 'https://provider.example.com/discovery',
+      })
+    )
+  })
+})
@@ -298,14 +298,23 @@ export function OAuthSection(props: OAuthSectionProps) {

  const onSubmit = async (values: OAuthFormValues) => {
    let finalValues = values
+    const wellKnown = values.oidc.well_known.trim()
+    const discoveryUrlChanged =
+      wellKnown !== (baselineRef.current['oidc.well_known'] ?? '').trim()
+    const enablingOIDC =
+      values.oidc.enabled && !baselineRef.current['oidc.enabled']

-    if (values.oidc.well_known && values.oidc.well_known.trim() !== '') {
-      const wellKnown = values.oidc.well_known.trim()
+    // Other providers and disabling OIDC must not depend on OIDC discovery.
+    if (wellKnown && (discoveryUrlChanged || enablingOIDC)) {
      if (
        !wellKnown.startsWith('http://') &&
        !wellKnown.startsWith('https://')
      ) {
-        toast.error(t('Well-Known URL must start with http:// or https://'))
+        setActiveTab('oidc')
+        form.setError('oidc.well_known', {
+          type: 'validate',
+          message: t('Well-Known URL must start with http:// or https://'),
+        })
        return
      }

@@ -319,6 +328,7 @@ export function OAuthSection(props: OAuthSectionProps) {
          ...values,
          oidc: {
            ...values.oidc,
+            well_known: wellKnown,
            authorization_endpoint: authEndpoint,
            token_endpoint: tokenEndpoint,
            user_info_endpoint: userInfoEndpoint,
@@ -378,7 +388,7 @@ export function OAuthSection(props: OAuthSectionProps) {
            <SettingsPageFormActions
              onSave={form.handleSubmit(onSubmit)}
              onReset={handleReset}
-              isSaving={updateOption.isPending}
+              isSaving={form.formState.isSubmitting}
              isResetDisabled={!form.formState.isDirty}
            />
            <FormDirtyIndicator isDirty={form.formState.isDirty} />
@@ -6607,8 +6607,8 @@
    "Weighted by request count": "Pondéré par le nombre de requêtes",
    "Welcome back!": "Bienvenue de retour !",
    "Welcome to our New API...": "Bienvenue sur notre New API...",
-    "Well-Known URL": "URL bien connue",
-    "Well-Known URL must start with http:// or https://": "L'URL bien connue doit commencer par http:// ou https://",
+    "Well-Known URL": "URL Well-Known",
+    "Well-Known URL must start with http:// or https://": "L’URL Well-Known doit commencer par http:// ou https://",
    "What would you like to know?": "Que voulez-vous savoir ?",
    "When a token uses the auto group, the system tries groups from top to bottom until it finds an available group.": "Quand un jeton utilise le groupe auto, le système essaie les groupes de haut en bas jusqu’à trouver un groupe disponible.",
    "When billed as {{group}}": "Facturé sous {{group}}",
@@ -6607,7 +6607,7 @@
    "Weighted by request count": "リクエスト数で加重",
    "Welcome back!": "おかえりなさい！",
    "Welcome to our New API...": "New API へようこそ...",
-    "Well-Known URL": "よく知られたURL",
+    "Well-Known URL": "Well-Known URL",
    "Well-Known URL must start with http:// or https://": "Well-Known URL は http:// または https:// で始まる必要があります",
    "What would you like to know?": "何を知りたいですか？",
    "When a token uses the auto group, the system tries groups from top to bottom until it finds an available group.": "トークンが auto グループを使用すると、システムは上から順に利用可能なグループを探します。",
@@ -6607,7 +6607,7 @@
    "Weighted by request count": "Взвешено по количеству запросов",
    "Welcome back!": "Добро пожаловать обратно!",
    "Welcome to our New API...": "Добро пожаловать в наш New API...",
-    "Well-Known URL": "Известный эксперт",
+    "Well-Known URL": "Well-Known URL",
    "Well-Known URL must start with http:// or https://": "Well-Known URL должен начинаться с http:// или https://",
    "What would you like to know?": "Что вы хотели бы узнать?",
    "When a token uses the auto group, the system tries groups from top to bottom until it finds an available group.": "Когда токен использует группу auto, система перебирает группы сверху вниз, пока не найдёт доступную.",
@@ -6607,7 +6607,7 @@
    "Weighted by request count": "Có trọng số theo số yêu cầu",
    "Welcome back!": "Chào mừng trở lại!",
    "Welcome to our New API...": "Chào mừng bạn đến với API mới của chúng tôi...",
-    "Well-Known URL": "URL đã biết",
+    "Well-Known URL": "URL Well-Known",
    "Well-Known URL must start with http:// or https://": "URL Well-Known phải bắt đầu bằng http:// hoặc https://",
    "What would you like to know?": "Bạn muốn biết gì?",
    "When a token uses the auto group, the system tries groups from top to bottom until it finds an available group.": "Khi token dùng nhóm auto, hệ thống thử các nhóm từ trên xuống dưới cho đến khi tìm được nhóm khả dụng.",
@@ -6608,7 +6608,7 @@
    "Welcome back!": "歡迎回來！",
    "Welcome to our New API...": "歡迎使用我們的 New API...",
    "Well-Known URL": "Well-Known URL",
-    "Well-Known URL must start with http:// or https://": "知名 URL 必須以 http:// 或 https:// 開頭",
+    "Well-Known URL must start with http:// or https://": "Well-Known URL 必須以 http:// 或 https:// 開頭",
    "What would you like to know?": "您想了解什麼？",
    "When a token uses the auto group, the system tries groups from top to bottom until it finds an available group.": "當令牌使用 auto 分組時，系統會按從上到下的順序嘗試，直到找到可用分組。",
    "When billed as {{group}}": "按 {{group}} 收費時",
@@ -6608,7 +6608,7 @@
    "Welcome back!": "欢迎回来！",
    "Welcome to our New API...": "欢迎使用我们的 New API...",
    "Well-Known URL": "Well-Known URL",
-    "Well-Known URL must start with http:// or https://": "知名 URL 必须以 http:// 或 https:// 开头",
+    "Well-Known URL must start with http:// or https://": "Well-Known URL 必须以 http:// 或 https:// 开头",
    "What would you like to know?": "您想了解什么？",
    "When a token uses the auto group, the system tries groups from top to bottom until it finds an available group.": "当令牌使用 auto 分组时，系统会按从上到下的顺序尝试，直到找到可用分组。",
    "When billed as {{group}}": "按 {{group}} 计费时",