From 2e780ffe7f34e8fb7e10eb935e3b6e6f2735b797 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Mon, 29 Jun 2026 21:18:04 +0800 Subject: [PATCH] fix(auth): normalize v2 signin redirects (#9927) * fix(auth): normalize v2 signin redirects * fix(auth): check redirect sub-app path safely * fix: detect pro plugin license paths * refactor(client-v2): simplify auth redirect fallback * refactor(client-v2): clarify auth redirect fallback * fix(auth): support configured app-sso issuer --- .../src/__tests__/authRedirect.test.ts | 17 ++++ packages/core/client-v2/src/authRedirect.ts | 38 +++++++++ .../app-supervisor/app-sso-issuer.test.ts | 33 ++++++++ .../core/server/src/app-supervisor/index.ts | 12 +++ .../client-v2/__tests__/SignInPage.test.tsx | 79 +++++++++++++++++++ .../src/client-v2/pages/SignInPage.tsx | 25 +++++- .../utils/__tests__/buildRedirectPath.test.ts | 15 ++++ .../src/server/utils/buildRedirectPath.ts | 12 +++ .../src/server/__tests__/service.test.ts | 21 +++++ .../plugin-idp-oauth/src/server/service.ts | 14 +++- scripts/addLicense.js | 4 +- 11 files changed, 264 insertions(+), 6 deletions(-) create mode 100644 packages/core/server/src/__tests__/app-supervisor/app-sso-issuer.test.ts create mode 100644 packages/plugins/@nocobase/plugin-auth/src/client-v2/__tests__/SignInPage.test.tsx diff --git a/packages/core/client-v2/src/__tests__/authRedirect.test.ts b/packages/core/client-v2/src/__tests__/authRedirect.test.ts index 71b79fa7a8c..5587e9dd033 100644 --- a/packages/core/client-v2/src/__tests__/authRedirect.test.ts +++ b/packages/core/client-v2/src/__tests__/authRedirect.test.ts @@ -10,6 +10,7 @@ import { buildV2SigninHref, getCurrentV2RedirectPath, + normalizeV2RedirectPath, redirectToV2Signin, resolveV2SigninRedirect, } from '../authRedirect'; @@ -133,6 +134,22 @@ describe('auth redirect helpers', () => { }); describe('v2 sub-app context (router basename contains /apps//)', () => { + it('should normalize signin redirect fallback under the current sub-app basename', () => { + const app = { + getPublicPath: () => '/v/', + router: { + getBasename: () => '/v/apps/test-app/', + }, + } as any; + + expect(normalizeV2RedirectPath(app, '')).toBe('/v/apps/test-app/admin/'); + expect(normalizeV2RedirectPath(app, '/admin/?tab=overview#panel')).toBe( + '/v/apps/test-app/admin/?tab=overview#panel', + ); + expect(normalizeV2RedirectPath(app, '/v/apps/test-app/admin/')).toBe('/v/apps/test-app/admin/'); + expect(normalizeV2RedirectPath(app, '/v/admin/')).toBe('/v/apps/test-app/admin/'); + }); + it('should preserve sub-app segment when building current redirect path under simple public path', () => { const app = { getPublicPath: () => '/v2/', diff --git a/packages/core/client-v2/src/authRedirect.ts b/packages/core/client-v2/src/authRedirect.ts index 600d94a3170..b38d4f21072 100644 --- a/packages/core/client-v2/src/authRedirect.ts +++ b/packages/core/client-v2/src/authRedirect.ts @@ -204,6 +204,44 @@ function getDefaultV2AdminRedirectPath(app: AppLike) { return joinRootRelativePath(getV2EffectiveBasePath(app), '/admin'); } +function isSafeRootRelativePath(value?: string | null) { + return !!value && value.startsWith('/') && !value.startsWith('//') && !value.startsWith('/\\'); +} + +function preserveTrailingSlash(originalPathname: string, value: string) { + if (originalPathname !== '/' && originalPathname.endsWith('/') && !value.endsWith('/')) { + return `${value}/`; + } + return value; +} + +export function normalizeV2RedirectPath(app: AppLike, target?: string | null, fallbackPath = '/admin/') { + // In a v2 sub-app, publicPath can be `/v/` while the active router + // basename is `/v/apps/a/`. Redirects must resolve under the basename. + const basePath = trimTrailingSlashes(getV2EffectiveBasePath(app)) || '/'; + const fallbackTarget = isSafeRootRelativePath(fallbackPath) ? fallbackPath : '/admin/'; + const rawTarget = isSafeRootRelativePath(target) ? target : fallbackTarget; + let { pathname, search, hash } = splitPathLike(rawTarget); + let normalizedPathname = normalizePathname(pathname); + + // Already under the current v2 runtime, e.g. `/v/apps/a/admin/`. + if (basePath === '/' || normalizedPathname === basePath || normalizedPathname.startsWith(`${basePath}/`)) { + return `${preserveTrailingSlash(pathname, normalizedPathname)}${normalizeSearch(search)}${normalizeHash(hash)}`; + } + + const publicPath = trimTrailingSlashes(getV2PublicPath(app)) || '/'; + // Under v2 publicPath but outside the current basename, e.g. `/v/admin/` + // inside `/v/apps/a/`; use fallback so it lands on `/v/apps/a/admin/`. + if (publicPath !== '/' && (normalizedPathname === publicPath || normalizedPathname.startsWith(`${publicPath}/`))) { + ({ pathname, search, hash } = splitPathLike(fallbackTarget)); + normalizedPathname = normalizePathname(pathname); + } + + // Basename-relative target, e.g. `/admin/`, becomes `/v/apps/a/admin/`. + const joinedPathname = preserveTrailingSlash(pathname, joinRootRelativePath(basePath, normalizedPathname)); + return `${joinedPathname}${normalizeSearch(search)}${normalizeHash(hash)}`; +} + /** * 将当前 v2 页面地址转换为根相对 redirect 路径。 * diff --git a/packages/core/server/src/__tests__/app-supervisor/app-sso-issuer.test.ts b/packages/core/server/src/__tests__/app-supervisor/app-sso-issuer.test.ts new file mode 100644 index 00000000000..135c87a365f --- /dev/null +++ b/packages/core/server/src/__tests__/app-supervisor/app-sso-issuer.test.ts @@ -0,0 +1,33 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import { AppSupervisor } from '../../app-supervisor'; + +describe('AppSupervisor app-sso issuer bridge', () => { + afterEach(async () => { + await AppSupervisor.getInstance().destroy(); + }); + + it('stores normalized app-sso issuer values', () => { + const supervisor = AppSupervisor.getInstance(); + + supervisor.setAppSsoIssuer(' https://main.example.com/api/ '); + + expect(supervisor.getAppSsoIssuer()).toBe('https://main.example.com/api'); + }); + + it('clears blank app-sso issuer values', () => { + const supervisor = AppSupervisor.getInstance(); + + supervisor.setAppSsoIssuer('https://main.example.com/api'); + supervisor.setAppSsoIssuer(' '); + + expect(supervisor.getAppSsoIssuer()).toBeUndefined(); + }); +}); diff --git a/packages/core/server/src/app-supervisor/index.ts b/packages/core/server/src/app-supervisor/index.ts index 93c1ab1aee9..e0efc00232a 100644 --- a/packages/core/server/src/app-supervisor/index.ts +++ b/packages/core/server/src/app-supervisor/index.ts @@ -75,6 +75,7 @@ export class AppSupervisor extends EventEmitter implements AsyncEmitter { private commandAdapterName: string; private appDbCreator = new ConditionalRegistry(); private appConditions = new Map(); + private appSsoIssuer?: string; public appOptionsFactory: AppOptionsFactory = appOptionsFactory; private environmentHeartbeatInterval = 2 * 60 * 1000; @@ -296,6 +297,17 @@ export class AppSupervisor extends EventEmitter implements AsyncEmitter { this.appOptionsFactory = factory ?? appOptionsFactory; } + setAppSsoIssuer(issuer?: string) { + const normalized = String(issuer || '') + .trim() + .replace(/\/+$/, ''); + this.appSsoIssuer = normalized || undefined; + } + + getAppSsoIssuer() { + return this.appSsoIssuer; + } + async bootstrapApp(appName: string) { return this.processAdapter.bootstrapApp(appName); } diff --git a/packages/plugins/@nocobase/plugin-auth/src/client-v2/__tests__/SignInPage.test.tsx b/packages/plugins/@nocobase/plugin-auth/src/client-v2/__tests__/SignInPage.test.tsx new file mode 100644 index 00000000000..4752bde6ac4 --- /dev/null +++ b/packages/plugins/@nocobase/plugin-auth/src/client-v2/__tests__/SignInPage.test.tsx @@ -0,0 +1,79 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import { render } from '@testing-library/react'; +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import SignInPage from '../pages/SignInPage'; + +const navigateMock = vi.fn(); +const mockApp = vi.hoisted(() => ({ + publicPath: '/v/', + basename: '/v/apps/sub/', +})); + +vi.mock('react-router-dom', async () => { + const actual = await vi.importActual('react-router-dom'); + return { + ...actual, + useNavigate: () => navigateMock, + }; +}); + +vi.mock('@nocobase/client-v2', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useApp: () => ({ + getPublicPath: () => mockApp.publicPath, + router: { + getBasename: () => mockApp.basename, + }, + }), + usePlugin: () => ({ + authTypes: { + getEntities: () => [], + }, + }), + }; +}); + +describe('SignInPage', () => { + beforeEach(() => { + navigateMock.mockReset(); + mockApp.publicPath = '/v/'; + mockApp.basename = '/v/apps/sub/'; + }); + + it('normalizes empty redirect to the current v2 app admin path', () => { + render( + + + , + ); + + expect(navigateMock).toHaveBeenCalledWith( + { + pathname: '/signin', + search: '?redirect=%2Fv%2Fapps%2Fsub%2Fadmin%2F', + }, + { replace: true }, + ); + }); + + it('keeps redirect that is already under the current v2 app basename', () => { + render( + + + , + ); + + expect(navigateMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/plugins/@nocobase/plugin-auth/src/client-v2/pages/SignInPage.tsx b/packages/plugins/@nocobase/plugin-auth/src/client-v2/pages/SignInPage.tsx index 94ca22d53bc..4f69db45bdd 100644 --- a/packages/plugins/@nocobase/plugin-auth/src/client-v2/pages/SignInPage.tsx +++ b/packages/plugins/@nocobase/plugin-auth/src/client-v2/pages/SignInPage.tsx @@ -7,9 +7,10 @@ * For more information, please refer to: https://www.nocobase.com/agreement. */ -import { usePlugin } from '@nocobase/client-v2'; +import { normalizeV2RedirectPath, useApp, usePlugin } from '@nocobase/client-v2'; import { Empty, Space, Spin, Tabs } from 'antd'; -import React, { lazy, Suspense, useContext, useMemo } from 'react'; +import React, { lazy, Suspense, useContext, useEffect, useMemo } from 'react'; +import { useLocation, useNavigate } from 'react-router-dom'; import { AuthenticatorsContext, type Authenticator } from '../authenticator'; import { useDocumentTitle } from '../hooks'; import { useAuthTranslation, useT } from '../locale'; @@ -43,6 +44,9 @@ function lazyByAuthType

(loaderMap: LoaderMap<() => Promise<{ default: React.C } export default function SignInPage() { + const app = useApp(); + const location = useLocation(); + const navigate = useNavigate(); const { t } = useAuthTranslation(); // `authTypeTitle` 从服务端来时是 `tval` 生成的原始模板字符串 // （`{{t("Password", {"ns":"@nocobase/plugin-auth"})}}`），不展开就会直出到 tab label @@ -57,6 +61,23 @@ export default function SignInPage() { const resolveSignInButton = useMemo(() => lazyByAuthType(signInButtonLoaders), [signInButtonLoaders]); useDocumentTitle(t('Signin')); + useEffect(() => { + const params = new URLSearchParams(location.search); + const redirect = params.get('redirect'); + const normalized = normalizeV2RedirectPath(app, redirect); + if (redirect === normalized) { + return; + } + + params.set('redirect', normalized); + navigate( + { + pathname: location.pathname, + search: `?${params}`, + }, + { replace: true }, + ); + }, [app, location.pathname, location.search, navigate]); const tabs = useMemo(() => { return authenticators diff --git a/packages/plugins/@nocobase/plugin-auth/src/server/utils/__tests__/buildRedirectPath.test.ts b/packages/plugins/@nocobase/plugin-auth/src/server/utils/__tests__/buildRedirectPath.test.ts index d9fa327d028..eb6d3e2590c 100644 --- a/packages/plugins/@nocobase/plugin-auth/src/server/utils/__tests__/buildRedirectPath.test.ts +++ b/packages/plugins/@nocobase/plugin-auth/src/server/utils/__tests__/buildRedirectPath.test.ts @@ -89,6 +89,21 @@ describe('buildRedirectPath', () => { ).toBe('/nocobase/v2/apps/a_u4940c6p189/admin/al5yj9t81of'); }); + it('does NOT prepend sub-app segment when target already contains it', () => { + expect(buildRedirectPath({ appPublicPath: '', subAppSegment: '/apps/sub', target: '/v/apps/sub/admin' })).toBe( + '/v/apps/sub/admin', + ); + expect( + buildRedirectPath({ appPublicPath: '', subAppSegment: '/apps/sub', target: '/v/apps/sub/admin?tab=x#panel' }), + ).toBe('/v/apps/sub/admin?tab=x#panel'); + }); + + it('does NOT treat query strings containing the sub-app segment as an existing sub-app path', () => { + expect( + buildRedirectPath({ appPublicPath: '', subAppSegment: '/apps/sub', target: '/admin?next=/apps/sub/foo' }), + ).toBe('/apps/sub/admin?next=/apps/sub/foo'); + }); + it('does NOT touch a v2 main-app target even when a sub-app segment was supplied', () => { // Detection is appPublicPath-only; supplying subAppSegment must not // override the v2-detection branch. diff --git a/packages/plugins/@nocobase/plugin-auth/src/server/utils/buildRedirectPath.ts b/packages/plugins/@nocobase/plugin-auth/src/server/utils/buildRedirectPath.ts index 9c435e4d81e..ea78802b6fa 100644 --- a/packages/plugins/@nocobase/plugin-auth/src/server/utils/buildRedirectPath.ts +++ b/packages/plugins/@nocobase/plugin-auth/src/server/utils/buildRedirectPath.ts @@ -53,6 +53,7 @@ export function buildRedirectPath({ appPublicPath, subAppSegment, target }: Buil const normalizedAppPublicPath = (appPublicPath || '').replace(/\/+$/, ''); const normalizedSubAppSegment = (subAppSegment || '').replace(/\/+$/, ''); const resolvedTarget = target || '/admin'; + const [resolvedPathname] = resolvedTarget.split(/[?#]/, 1); if ( normalizedAppPublicPath && @@ -61,6 +62,17 @@ export function buildRedirectPath({ appPublicPath, subAppSegment, target }: Buil return resolvedTarget; } + const modernSubAppSegment = `/${getModernClientPrefix()}${normalizedSubAppSegment}`; + if ( + normalizedSubAppSegment && + (resolvedPathname === normalizedSubAppSegment || + resolvedPathname.startsWith(`${normalizedSubAppSegment}/`) || + resolvedPathname === modernSubAppSegment || + resolvedPathname.startsWith(`${modernSubAppSegment}/`)) + ) { + return resolvedTarget; + } + return `${normalizedAppPublicPath}${normalizedSubAppSegment}${resolvedTarget}`; } diff --git a/packages/plugins/@nocobase/plugin-idp-oauth/src/server/__tests__/service.test.ts b/packages/plugins/@nocobase/plugin-idp-oauth/src/server/__tests__/service.test.ts index 41d7680fadf..9de8f9d5293 100644 --- a/packages/plugins/@nocobase/plugin-idp-oauth/src/server/__tests__/service.test.ts +++ b/packages/plugins/@nocobase/plugin-idp-oauth/src/server/__tests__/service.test.ts @@ -96,6 +96,27 @@ describe('plugin-idp-oauth > IdpOauthService', () => { ); }); + test('should use the first forwarded header value when proxies append duplicates', () => { + const service = new IdpOauthService({} as any, {} as any); + vi.spyOn(AppSupervisor, 'getInstance').mockReturnValue({ + runningMode: 'multiple', + } as any); + + const providerContext = service.getProviderContext({ + app: { name: 'subapp' }, + path: '/api/app:getLang', + protocol: 'http', + host: '127.0.0.1:13001', + headers: { + 'x-forwarded-proto': 'http,http', + 'x-forwarded-host': 'app.noco.local', + }, + }); + + expect(providerContext.origin).toBe('http://app.noco.local'); + expect(providerContext.issuer).toBe('http://app.noco.local/api'); + }); + test('should preserve sub app issuer path when original url uses __app prefix', () => { const service = new IdpOauthService({} as any, {} as any); vi.spyOn(AppSupervisor, 'getInstance').mockReturnValue({ diff --git a/packages/plugins/@nocobase/plugin-idp-oauth/src/server/service.ts b/packages/plugins/@nocobase/plugin-idp-oauth/src/server/service.ts index ff8b848b85e..fc67e0a2736 100644 --- a/packages/plugins/@nocobase/plugin-idp-oauth/src/server/service.ts +++ b/packages/plugins/@nocobase/plugin-idp-oauth/src/server/service.ts @@ -51,6 +51,14 @@ function getJoseModule() { return joseModulePromise; } +function getHeaderValue(value: unknown) { + const raw = Array.isArray(value) ? value[0] : value; + if (typeof raw !== 'string') { + return undefined; + } + return raw.split(',')[0]?.trim() || undefined; +} + export type ResourceServerConfig = { path?: string; identifier?: string; @@ -255,8 +263,8 @@ export class IdpOauthService { } getOrigin(ctx: any) { - const protocol = ctx.headers?.['x-forwarded-proto'] || ctx.protocol || 'http'; - const host = ctx.headers?.['x-forwarded-host'] || ctx.host || ''; + const protocol = getHeaderValue(ctx.headers?.['x-forwarded-proto']) || ctx.protocol || 'http'; + const host = getHeaderValue(ctx.headers?.['x-forwarded-host']) || ctx.host || ''; return process.env.APP_PUBLIC_ORIGIN || `${protocol}://${host}`; } @@ -644,7 +652,7 @@ export class IdpOauthService { ctx.cookies?.set?.(provider.cookieName('session'), null, { httpOnly: true, sameSite: 'lax', - secure: (ctx.headers?.['x-forwarded-proto'] || ctx.protocol) === 'https', + secure: (getHeaderValue(ctx.headers?.['x-forwarded-proto']) || ctx.protocol) === 'https', }); } diff --git a/scripts/addLicense.js b/scripts/addLicense.js index 29a38913335..e63ecfddc81 100644 --- a/scripts/addLicense.js +++ b/scripts/addLicense.js @@ -1,5 +1,6 @@ const fs = require('fs/promises'); const { exec } = require('child_process'); +const path = require('path'); const commercialLicense = ` /** @@ -27,11 +28,12 @@ function getLicenseText(packageDir) { } async function addLicenseToFile(filePath) { - const licenseText = getLicenseText(filePath); + const licenseText = getLicenseText(path.resolve(filePath)); const data = await fs.readFile(filePath, 'utf8'); if (data.startsWith(licenseText)) return false; + if (data.startsWith(commercialLicense) || data.startsWith(openSourceLicense)) return false; // 添加授权信息到文件内容的顶部 const newData = licenseText + '\n\n' + data;