From ea6770c7a1fcaac14711e626d01b4661139c1e17 Mon Sep 17 00:00:00 2001 From: L1nSn0w Date: Fri, 14 Aug 2026 10:29:39 +0000 Subject: [PATCH] feat(oauth): drive silent authorization from oauth_provider_apps.auto_authorize (#40783) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- api/controllers/console/auth/oauth_server.py | 2 + ...e_add_oauth_provider_app_auto_authorize.py | 34 ++++ api/models/model.py | 4 + api/openapi/markdown/console-openapi.md | 1 + .../console/auth/test_oauth_server.py | 29 +++- .../generated/api/console/oauth/types.gen.ts | 1 + .../generated/api/console/oauth/zod.gen.ts | 1 + .../oauth/authorize/__tests__/page.spec.tsx | 147 +++++++++++++++--- .../__tests__/use-silent-authorize.spec.ts | 36 +++++ web/app/account/oauth/authorize/page.tsx | 28 ++-- .../oauth/authorize/use-silent-authorize.ts | 93 +++++++++++ 11 files changed, 338 insertions(+), 38 deletions(-) create mode 100644 api/migrations/versions/2026_08_14_1000-f3a9c2d17b4e_add_oauth_provider_app_auto_authorize.py create mode 100644 web/app/account/oauth/authorize/__tests__/use-silent-authorize.spec.ts create mode 100644 web/app/account/oauth/authorize/use-silent-authorize.ts diff --git a/api/controllers/console/auth/oauth_server.py b/api/controllers/console/auth/oauth_server.py index 7eb043b2fd1..fe297c6c725 100644 --- a/api/controllers/console/auth/oauth_server.py +++ b/api/controllers/console/auth/oauth_server.py @@ -42,6 +42,7 @@ class OAuthProviderAppResponse(BaseModel): app_icon: str app_label: dict[str, Any] scope: str + auto_authorize: bool class OAuthProviderAuthorizeResponse(BaseModel): @@ -167,6 +168,7 @@ class OAuthServerAppApi(Resource): "app_icon": oauth_provider_app.app_icon, "app_label": oauth_provider_app.app_label, "scope": oauth_provider_app.scope, + "auto_authorize": oauth_provider_app.auto_authorize, } ) diff --git a/api/migrations/versions/2026_08_14_1000-f3a9c2d17b4e_add_oauth_provider_app_auto_authorize.py b/api/migrations/versions/2026_08_14_1000-f3a9c2d17b4e_add_oauth_provider_app_auto_authorize.py new file mode 100644 index 00000000000..8f00fba90a8 --- /dev/null +++ b/api/migrations/versions/2026_08_14_1000-f3a9c2d17b4e_add_oauth_provider_app_auto_authorize.py @@ -0,0 +1,34 @@ +"""add oauth provider app auto_authorize + +Revision ID: f3a9c2d17b4e +Revises: a1c7f4e9b3d2 +Create Date: 2026-08-14 10:00:00.000000 + +Adds `oauth_provider_apps.auto_authorize`: first-party apps (e.g. the Dify +Marketplace) whose consent screen is skipped. The flag is only a rendering +hint returned by `POST /console/api/oauth/provider`; issuing an authorization +code still requires a logged-in console session. + +DDL only. `server_default=false` backfills every existing row, so all +registered apps keep the consent-screen behavior until explicitly opted in. + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "f3a9c2d17b4e" +down_revision = "a1c7f4e9b3d2" +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table("oauth_provider_apps", schema=None) as batch_op: + batch_op.add_column(sa.Column("auto_authorize", sa.Boolean(), nullable=False, server_default=sa.false())) + + +def downgrade(): + with op.batch_alter_table("oauth_provider_apps", schema=None) as batch_op: + batch_op.drop_column("auto_authorize") diff --git a/api/models/model.py b/api/models/model.py index 0a09f50b844..e439b54bda1 100644 --- a/api/models/model.py +++ b/api/models/model.py @@ -1154,6 +1154,10 @@ class OAuthProviderApp(TypeBase): server_default=sa.text("'read:name read:email read:avatar read:interface_language read:timezone'"), default="read:name read:email read:avatar read:interface_language read:timezone", ) + # First-party apps (e.g. the Dify Marketplace) skip the consent screen. + # Default false: self-hosted / EE / newly registered apps keep the + # consent-screen behavior. + auto_authorize: Mapped[bool] = mapped_column(sa.Boolean, nullable=False, server_default=sa.false(), default=False) created_at: Mapped[datetime] = mapped_column( sa.DateTime, nullable=False, server_default=func.current_timestamp(), init=False ) diff --git a/api/openapi/markdown/console-openapi.md b/api/openapi/markdown/console-openapi.md index e13541c60e1..9fff14ab3b1 100644 --- a/api/openapi/markdown/console-openapi.md +++ b/api/openapi/markdown/console-openapi.md @@ -19778,6 +19778,7 @@ Coarse node-level status used by Inspector to pick a banner. | ---- | ---- | ----------- | -------- | | app_icon | string | | Yes | | app_label | object | | Yes | +| auto_authorize | boolean | | Yes | | scope | string | | Yes | #### OAuthProviderAuthorizeResponse diff --git a/api/tests/unit_tests/controllers/console/auth/test_oauth_server.py b/api/tests/unit_tests/controllers/console/auth/test_oauth_server.py index ab5534c121c..7a03e089e9d 100644 --- a/api/tests/unit_tests/controllers/console/auth/test_oauth_server.py +++ b/api/tests/unit_tests/controllers/console/auth/test_oauth_server.py @@ -3,7 +3,13 @@ from __future__ import annotations from inspect import unwrap from unittest.mock import patch -from controllers.console.auth.oauth_server import OAuthServerUserAccountApi, OAuthServerUserAuthorizeApi +from controllers.console.auth.oauth_server import ( + OAuthProviderAppResponse, + OAuthProviderRequest, + OAuthServerAppApi, + OAuthServerUserAccountApi, + OAuthServerUserAuthorizeApi, +) from models import Account from models.account import AccountStatus, TenantAccountRole from models.model import OAuthProviderApp @@ -55,3 +61,24 @@ def test_oauth_account_returns_stable_account_id() -> None: response = method(api, _make_oauth_provider_app(), account) assert response["id"] == "account-1" + + +def test_oauth_provider_app_response_requires_auto_authorize() -> None: + # A missing field must fail validation instead of silently defaulting: + # an optional field would surface as `undefined` in the generated TS + # contract and silently disable silent authorization. + assert "auto_authorize" in OAuthProviderAppResponse.model_json_schema()["required"] + + +def test_oauth_provider_returns_auto_authorize_flag() -> None: + api = OAuthServerAppApi() + method = unwrap(api.post) + payload = OAuthProviderRequest(client_id="client-1", redirect_uri="https://example.com/callback") + + response = method(api, payload, _make_oauth_provider_app()) + assert response["auto_authorize"] is False + + opted_in = _make_oauth_provider_app() + opted_in.auto_authorize = True + response = method(api, payload, opted_in) + assert response["auto_authorize"] is True diff --git a/packages/contracts/generated/api/console/oauth/types.gen.ts b/packages/contracts/generated/api/console/oauth/types.gen.ts index 322e4ca74fb..2108c56280d 100644 --- a/packages/contracts/generated/api/console/oauth/types.gen.ts +++ b/packages/contracts/generated/api/console/oauth/types.gen.ts @@ -30,6 +30,7 @@ export type OAuthProviderAppResponse = { app_label: { [key: string]: unknown } + auto_authorize: boolean scope: string } diff --git a/packages/contracts/generated/api/console/oauth/zod.gen.ts b/packages/contracts/generated/api/console/oauth/zod.gen.ts index 2e2523a8b8a..960d1e429c3 100644 --- a/packages/contracts/generated/api/console/oauth/zod.gen.ts +++ b/packages/contracts/generated/api/console/oauth/zod.gen.ts @@ -44,6 +44,7 @@ export const zOAuthProviderRequest = z.object({ export const zOAuthProviderAppResponse = z.object({ app_icon: z.string(), app_label: z.record(z.string(), z.unknown()), + auto_authorize: z.boolean(), scope: z.string(), }) diff --git a/web/app/account/oauth/authorize/__tests__/page.spec.tsx b/web/app/account/oauth/authorize/__tests__/page.spec.tsx index e5d1194330c..a06d4499e46 100644 --- a/web/app/account/oauth/authorize/__tests__/page.spec.tsx +++ b/web/app/account/oauth/authorize/__tests__/page.spec.tsx @@ -6,28 +6,30 @@ import { seedSystemFeatures } from '@/test/console/query-data' import OAuthAuthorize from '../page' const mocks = vi.hoisted(() => ({ + profileLoggedIn: true, push: vi.fn(), + replace: vi.fn(), request: vi.fn(), searchParams: new URLSearchParams(), })) vi.mock('@/next/navigation', () => ({ - useRouter: () => ({ push: mocks.push }), + useRouter: () => ({ push: mocks.push, replace: mocks.replace }), useSearchParams: () => mocks.searchParams, })) vi.mock('@/service/base', () => ({ - get: vi.fn( - async () => - new Response( - JSON.stringify({ - avatar_url: null, - email: 'user@example.com', - name: 'Test User', - }), - { status: 200 }, - ), - ), + get: vi.fn(async () => { + if (!mocks.profileLoggedIn) throw new Response(null, { status: 401 }) + return new Response( + JSON.stringify({ + avatar_url: null, + email: 'user@example.com', + name: 'Test User', + }), + { status: 200 }, + ) + }), post: vi.fn(), request: (...args: unknown[]) => mocks.request(...args), sseGeneratorPost: vi.fn(), @@ -60,25 +62,35 @@ function findRequest(path: string) { return mocks.request.mock.calls.find(([url]) => String(url).endsWith(path)) } +function mockProviderResponses({ autoAuthorize }: { autoAuthorize: boolean }) { + mocks.request.mockImplementation(async (url: string) => { + if (url.endsWith('/oauth/provider/authorize')) return jsonResponse({ code: 'oauth-code' }) + if (url.endsWith('/oauth/provider')) { + return jsonResponse({ + app_icon: '', + app_label: { en_US: 'Test OAuth App' }, + auto_authorize: autoAuthorize, + scope: '', + }) + } + throw new Error(`Unexpected request: ${url}`) + }) +} + +function countRequests(path: string) { + return mocks.request.mock.calls.filter(([url]) => String(url).endsWith(path)).length +} + describe('OAuthAuthorize', () => { beforeEach(() => { vi.clearAllMocks() + mocks.profileLoggedIn = true mocks.searchParams = new URLSearchParams({ client_id: 'client-1', redirect_uri: 'https://client.example.com/callback', state: 'state-1', }) - mocks.request.mockImplementation(async (url: string) => { - if (url.endsWith('/oauth/provider/authorize')) return jsonResponse({ code: 'oauth-code' }) - if (url.endsWith('/oauth/provider')) { - return jsonResponse({ - app_icon: '', - app_label: { en_US: 'Test OAuth App' }, - scope: '', - }) - } - throw new Error(`Unexpected request: ${url}`) - }) + mockProviderResponses({ autoAuthorize: false }) vi.stubGlobal('location', { href: 'https://dify.test/account/oauth/authorize', origin: 'https://dify.test', @@ -114,4 +126,93 @@ describe('OAuthAuthorize', () => { ), ) }) + + it('silently authorizes an app flagged with auto_authorize without rendering consent', async () => { + mocks.searchParams = new URLSearchParams({ + client_id: 'marketplace-client', + redirect_uri: 'https://api.marketplace.example.com/api/v1/auth/callback/dify', + response_type: 'code', + state: 'marketplace-state', + }) + mockProviderResponses({ autoAuthorize: true }) + + renderPage() + + await waitFor(() => + expect(globalThis.location.href).toBe( + 'https://api.marketplace.example.com/api/v1/auth/callback/dify?code=oauth-code&state=marketplace-state', + ), + ) + expect(countRequests('/oauth/provider/authorize')).toBe(1) + expect(screen.queryByRole('button', { name: /continue/i })).not.toBeInTheDocument() + }) + + it('keeps the consent flow when the app is not flagged with auto_authorize', async () => { + renderPage() + + expect(await screen.findByRole('button', { name: /continue/i })).toBeInTheDocument() + expect(findRequest('/oauth/provider/authorize')).toBeUndefined() + }) + + it('sends an anonymous user of an auto_authorize app to signin with the full authorize URL', async () => { + mocks.profileLoggedIn = false + mocks.searchParams = new URLSearchParams({ + client_id: 'marketplace-client', + redirect_uri: 'https://api.marketplace.example.com/api/v1/auth/callback/dify', + response_type: 'code', + state: 'marketplace-state', + }) + mockProviderResponses({ autoAuthorize: true }) + + renderPage() + + await waitFor(() => + expect(mocks.replace).toHaveBeenCalledWith( + `/signin?redirect_url=${encodeURIComponent( + 'https://dify.test/account/oauth/authorize?client_id=marketplace-client&redirect_uri=https%3A%2F%2Fapi.marketplace.example.com%2Fapi%2Fv1%2Fauth%2Fcallback%2Fdify&response_type=code&state=marketplace-state', + )}`, + ), + ) + expect(findRequest('/oauth/provider')).toBeDefined() + expect(findRequest('/oauth/provider/authorize')).toBeUndefined() + }) + + it('falls back to manual confirmation when silent authorization fails', async () => { + mocks.searchParams = new URLSearchParams({ + client_id: 'marketplace-client', + redirect_uri: 'https://api.marketplace.example.com/api/v1/auth/callback/dify', + response_type: 'code', + state: 'marketplace-state', + }) + let authorizeAttempts = 0 + mocks.request.mockImplementation(async (url: string) => { + if (url.endsWith('/oauth/provider/authorize')) { + authorizeAttempts += 1 + if (authorizeAttempts === 1) throw new Error('Automatic authorization failed') + return jsonResponse({ code: 'oauth-code' }) + } + if (url.endsWith('/oauth/provider')) { + return jsonResponse({ + app_icon: '', + app_label: { en_US: 'Test OAuth App' }, + auto_authorize: true, + scope: '', + }) + } + throw new Error(`Unexpected request: ${url}`) + }) + + const user = userEvent.setup() + renderPage() + + const continueButton = await screen.findByRole('button', { name: /continue/i }) + await user.click(continueButton) + + await waitFor(() => expect(authorizeAttempts).toBe(2)) + await waitFor(() => + expect(globalThis.location.href).toBe( + 'https://api.marketplace.example.com/api/v1/auth/callback/dify?code=oauth-code&state=marketplace-state', + ), + ) + }) }) diff --git a/web/app/account/oauth/authorize/__tests__/use-silent-authorize.spec.ts b/web/app/account/oauth/authorize/__tests__/use-silent-authorize.spec.ts new file mode 100644 index 00000000000..d42fc7a2171 --- /dev/null +++ b/web/app/account/oauth/authorize/__tests__/use-silent-authorize.spec.ts @@ -0,0 +1,36 @@ +import { afterEach, describe, expect, it, vi } from 'vite-plus/test' +import { buildOAuthCallbackUrl, buildReturnUrl } from '../use-silent-authorize' + +describe('buildReturnUrl', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('prefixes the current origin', () => { + vi.stubGlobal('location', { origin: 'https://dify.test' }) + + expect(buildReturnUrl('/account/oauth/authorize', '?a=1')).toBe( + 'https://dify.test/account/oauth/authorize?a=1', + ) + }) + + it('falls back to a relative URL when location is unavailable', () => { + vi.stubGlobal('location', undefined) + + expect(buildReturnUrl('/account/oauth/authorize', '?a=1')).toBe('/account/oauth/authorize?a=1') + }) +}) + +describe('buildOAuthCallbackUrl', () => { + it('appends code and state to the redirect URI', () => { + expect(buildOAuthCallbackUrl('https://client.example.com/callback', 'code-1', 'state-1')).toBe( + 'https://client.example.com/callback?code=code-1&state=state-1', + ) + }) + + it('omits a null state', () => { + expect(buildOAuthCallbackUrl('https://client.example.com/callback', 'code-1', null)).toBe( + 'https://client.example.com/callback?code=code-1', + ) + }) +}) diff --git a/web/app/account/oauth/authorize/page.tsx b/web/app/account/oauth/authorize/page.tsx index 21bcb42fe8b..8b941e461d6 100644 --- a/web/app/account/oauth/authorize/page.tsx +++ b/web/app/account/oauth/authorize/page.tsx @@ -21,15 +21,7 @@ import useDocumentTitle from '@/hooks/use-document-title' import { useRouter, useSearchParams } from '@/next/navigation' import { consoleQuery } from '@/service/client' import { useLogout } from '@/service/use-common' - -function buildReturnUrl(pathname: string, search: string) { - try { - const base = `${globalThis.location.origin}${pathname}${search}` - return base - } catch { - return pathname + search - } -} +import { buildOAuthCallbackUrl, buildReturnUrl, useSilentAuthorize } from './use-silent-authorize' export default function OAuthAuthorize() { const { t } = useTranslation() @@ -94,6 +86,17 @@ export default function OAuthAuthorize() { consoleQuery.oauth.provider.authorize.post.mutationOptions(), ) const { mutateAsync: logout } = useLogout() + const { isAutoAuthorizing } = useSilentAuthorize({ + authAppInfo, + authorize, + clientId: client_id, + hasOAuthParams, + isLoggedIn, + isProfileLoading, + redirectUri: redirect_uri, + searchParams, + state, + }) const hasNotifiedRef = useRef(false) const localizedAppLabel = authAppInfo?.app_label[language] const englishAppLabel = authAppInfo?.app_label.en_US @@ -122,10 +125,7 @@ export default function OAuthAuthorize() { if (!client_id || !redirect_uri) return try { const { code } = await authorize({ body: { client_id } }) - const url = new URL(redirect_uri) - url.searchParams.set('code', code) - if (state) url.searchParams.set('state', state) - globalThis.location.href = url.toString() + globalThis.location.href = buildOAuthCallbackUrl(redirect_uri, code, state) } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error) toast.error(`${t(($) => $['error.authorizeFailed'], { ns: 'oauth' })}: ${message}`) @@ -145,7 +145,7 @@ export default function OAuthAuthorize() { } }, [client_id, redirect_uri, isError]) - if (isLoading) { + if (isLoading || isAutoAuthorizing) { return (
diff --git a/web/app/account/oauth/authorize/use-silent-authorize.ts b/web/app/account/oauth/authorize/use-silent-authorize.ts new file mode 100644 index 00000000000..6db8355072b --- /dev/null +++ b/web/app/account/oauth/authorize/use-silent-authorize.ts @@ -0,0 +1,93 @@ +import { toast } from '@langgenius/dify-ui/toast' +import { useEffect, useRef, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { useRouter } from '@/next/navigation' + +export function buildReturnUrl(pathname: string, search: string) { + try { + return `${globalThis.location.origin}${pathname}${search}` + } catch { + return pathname + search + } +} + +export function buildOAuthCallbackUrl(redirectUri: string, code: string, state: string | null) { + const url = new URL(redirectUri) + url.searchParams.set('code', code) + if (state) url.searchParams.set('state', state) + return url.toString() +} + +type SilentAuthorizeOptions = { + authAppInfo: { auto_authorize: boolean } | undefined + authorize: (input: { body: { client_id: string } }) => Promise<{ code: string }> + clientId: string + hasOAuthParams: boolean + isLoggedIn: boolean + isProfileLoading: boolean + redirectUri: string + searchParams: { toString: () => string } + state: string | null +} + +/** + * Skips the consent screen for first-party apps (e.g. the Dify Marketplace) + * flagged with `auto_authorize` on their `oauth_provider_apps` row, as + * returned by `POST /oauth/provider`. The flag is only a rendering hint: + * issuing an authorization code still requires a logged-in console session, + * so a tampered response only affects the tamperer's own UI. + */ +export function useSilentAuthorize({ + authAppInfo, + authorize, + clientId, + hasOAuthParams, + isLoggedIn, + isProfileLoading, + redirectUri, + searchParams, + state, +}: SilentAuthorizeOptions) { + const { t } = useTranslation() + const router = useRouter() + const startedRef = useRef(false) + const [autoAuthorizationFailed, setAutoAuthorizationFailed] = useState(false) + const shouldAutoAuthorize = hasOAuthParams && Boolean(authAppInfo?.auto_authorize) + + useEffect(() => { + if (!shouldAutoAuthorize || startedRef.current || isProfileLoading) return + + if (!isLoggedIn) { + startedRef.current = true + const returnUrl = buildReturnUrl('/account/oauth/authorize', `?${searchParams.toString()}`) + router.replace(`/signin?redirect_url=${encodeURIComponent(returnUrl)}`) + return + } + + startedRef.current = true + void authorize({ body: { client_id: clientId } }) + .then(({ code }) => { + globalThis.location.href = buildOAuthCallbackUrl(redirectUri, code, state) + }) + .catch((error: unknown) => { + setAutoAuthorizationFailed(true) + const message = error instanceof Error ? error.message : String(error) + toast.error(`${t(($) => $['error.authorizeFailed'], { ns: 'oauth' })}: ${message}`) + }) + }, [ + authorize, + clientId, + isLoggedIn, + isProfileLoading, + redirectUri, + router, + searchParams, + shouldAutoAuthorize, + state, + t, + ]) + + return { + isAutoAuthorizing: shouldAutoAuthorize && !autoAuthorizationFailed, + } +}