diff --git a/api/services/enterprise/rbac_service.py b/api/services/enterprise/rbac_service.py index 5f206548cc5..6089cc2237e 100644 --- a/api/services/enterprise/rbac_service.py +++ b/api/services/enterprise/rbac_service.py @@ -475,6 +475,7 @@ _LEGACY_WORKSPACE_DATASET_OPERATOR_KEYS: list[str] = [ _LEGACY_APP_OWNER_KEYS: list[str] = [ "app.acl.preview", + "access_point.manage", "app.acl.view_layout", "app.acl.test_and_run", "app.acl.edit", @@ -490,6 +491,7 @@ _LEGACY_APP_OWNER_KEYS: list[str] = [ _LEGACY_APP_ADMIN_KEYS: list[str] = [ "app.acl.preview", "app.acl.view_layout", + "access_point.manage", "app.acl.test_and_run", "app.acl.edit", "app.acl.import_export_dsl", @@ -504,6 +506,7 @@ _LEGACY_APP_ADMIN_KEYS: list[str] = [ _LEGACY_APP_EDITOR_KEYS: list[str] = [ "app.acl.preview", + "access_point.manage", "app.acl.view_layout", "app.acl.test_and_run", "app.acl.edit", diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index 0b17e1b70df..dfbc8b6c8e8 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -1068,11 +1068,6 @@ "count": 2 } }, - "web/app/components/base/icons/src/public/billing/index.ts": { - "no-barrel-files/no-barrel-files": { - "count": 4 - } - }, "web/app/components/base/icons/src/public/common/index.ts": { "no-barrel-files/no-barrel-files": { "count": 5 @@ -1767,11 +1762,6 @@ "count": 3 } }, - "web/app/components/billing/pricing/assets/index.tsx": { - "no-barrel-files/no-barrel-files": { - "count": 12 - } - }, "web/app/components/datasets/chunk.tsx": { "jsx-a11y/label-has-associated-control": { "count": 2 @@ -5408,4 +5398,4 @@ "count": 2 } } -} +} \ No newline at end of file diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/__tests__/layout-main.spec.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/__tests__/layout-main.spec.tsx index 1fc6114bca5..41fad11f4d8 100644 --- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/__tests__/layout-main.spec.tsx +++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/__tests__/layout-main.spec.tsx @@ -234,9 +234,11 @@ describe('AppDetailLayout', () => { expect(useStore.getState().appDetail?.id).toBe('app-1') }) - it('should allow access point pages without app deploy or app ACL permissions', async () => { + it('should allow users with access point permission to open access point directly', async () => { mockPathname = '/app/app-1/access-point' - mockFetchAppDetailDirect.mockResolvedValue(createAppDetail({ permission_keys: [] })) + mockFetchAppDetailDirect.mockResolvedValue( + createAppDetail({ permission_keys: [AppACLPermission.AccessPoint] }), + ) render( @@ -250,6 +252,25 @@ describe('AppDetailLayout', () => { expect(useStore.getState().appDetail?.id).toBe('app-1') }) + it('should redirect access point pages when access point permission is missing', async () => { + mockPathname = '/app/app-1/access-point' + mockFetchAppDetailDirect.mockResolvedValue( + createAppDetail({ permission_keys: [AppACLPermission.Monitor] }), + ) + + render( + +
App page content
+
, + ) + + await waitFor(() => { + expect(mockReplace).toHaveBeenCalledWith('/app/app-1/overview') + }) + expect(screen.queryByText('App page content')).not.toBeInTheDocument() + expect(useStore.getState().appDetail).toBeUndefined() + }) + it('should redirect deploy pages when app deploy ACL permission is missing', async () => { mockPathname = '/app/app-1/deploy' mockFetchAppDetailDirect.mockResolvedValue( @@ -313,7 +334,7 @@ describe('AppDetailLayout', () => { ) await waitFor(() => { - expect(mockReplace).toHaveBeenCalledWith('/app/app-1/access-point') + expect(mockReplace).toHaveBeenCalledWith('/apps') }) expect(screen.queryByText('App page content')).not.toBeInTheDocument() expect(useStore.getState().appDetail).toBeUndefined() @@ -438,7 +459,7 @@ describe('AppDetailLayout', () => { ) await waitFor(() => { - expect(mockReplace).toHaveBeenCalledWith('/app/app-1/access-point') + expect(mockReplace).toHaveBeenCalledWith('/apps') }) expect(screen.queryByText('App page content')).not.toBeInTheDocument() expect(useStore.getState().appDetail).toBeUndefined() diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx index 24a254fe57e..bfd246e8e96 100644 --- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx +++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx @@ -140,6 +140,7 @@ const AppDetailLayout: FC = (props) => { const isOverviewPath = pathname.endsWith('overview') const isAccessConfigPath = pathname.endsWith('access-config') const isDeployPath = pathname.endsWith('deploy') + const isAccessPointPath = pathname.endsWith('access-point') if ( (isLayoutPath && !appACLCapabilities.canAccessLayout) || (isLogsPath && !appACLCapabilities.canAccessLogAndAnnotation) || @@ -147,7 +148,8 @@ const AppDetailLayout: FC = (props) => { (isOverviewPath && !appACLCapabilities.canMonitor) || (isAccessConfigPath && !appACLCapabilities.canAccessConfig) || (isDeployPath && - (routeAppDetail.mode !== AppModeEnum.WORKFLOW || !appACLCapabilities.canDeploy)) + (routeAppDetail.mode !== AppModeEnum.WORKFLOW || !appACLCapabilities.canDeploy)) || + (isAccessPointPath && !appACLCapabilities.canAccessPoint) ) { router.replace( getRedirectionPath(routeAppDetail, { diff --git a/web/app/components/app-sidebar/__tests__/app-detail-section.spec.tsx b/web/app/components/app-sidebar/__tests__/app-detail-section.spec.tsx index 3b8b116c0d0..9c5df3a6f41 100644 --- a/web/app/components/app-sidebar/__tests__/app-detail-section.spec.tsx +++ b/web/app/components/app-sidebar/__tests__/app-detail-section.spec.tsx @@ -185,7 +185,10 @@ describe('AppDetailSection', () => { ).not.toBeInTheDocument() }) - it('should render access point navigation using its app route', () => { + it('should render access point navigation when access point permission is granted', () => { + // Arrange + mockAppPermissionKeys = [AppACLPermission.AccessPoint] + // Act render() @@ -199,6 +202,19 @@ describe('AppDetailSection', () => { ).not.toBeInTheDocument() }) + it('should hide access point navigation when access point permission is missing', () => { + // Arrange + mockAppPermissionKeys = [AppACLPermission.Monitor] + + // Act + render() + + // Assert + expect( + screen.queryByRole('link', { name: 'common.appMenus.accessPoint' }), + ).not.toBeInTheDocument() + }) + it('should render deploy navigation with app deploy ACL regardless of the legacy workspace role', () => { // Arrange mockAppMode = 'workflow' diff --git a/web/app/components/app-sidebar/app-detail-section.tsx b/web/app/components/app-sidebar/app-detail-section.tsx index 505c8c15a45..1de6b10c92b 100644 --- a/web/app/components/app-sidebar/app-detail-section.tsx +++ b/web/app/components/app-sidebar/app-detail-section.tsx @@ -119,12 +119,16 @@ const AppDetailSection = ({ expand = true }: AppDetailSectionProps) => { }, ] : []), - { - name: t(($) => $['appMenus.accessPoint'], { ns: 'common' }), - href: `/app/${appId}/access-point`, - icon: accessPointNavIcon, - selectedIcon: accessPointNavIcon, - }, + ...(appACLCapabilities.canAccessPoint + ? [ + { + name: t(($) => $['appMenus.accessPoint'], { ns: 'common' }), + href: `/app/${appId}/access-point`, + icon: accessPointNavIcon, + selectedIcon: accessPointNavIcon, + }, + ] + : []), ...(supportsAppDeploy && appACLCapabilities.canDeploy ? [ { diff --git a/web/utils/app-redirection.spec.ts b/web/utils/app-redirection.spec.ts index d736ed5a428..521a500652d 100644 --- a/web/utils/app-redirection.spec.ts +++ b/web/utils/app-redirection.spec.ts @@ -14,12 +14,22 @@ describe('app-redirection', () => { * - App mode (workflow, advanced-chat, chat, completion, agent-chat) */ describe('getRedirectionPath', () => { - it('returns access point path when app ACL cannot access guarded pages', () => { - const app = { id: 'app-123', mode: AppModeEnum.CHAT, permission_keys: [] } + it('returns access point path when app access point permission is granted', () => { + const app = { + id: 'app-123', + mode: AppModeEnum.CHAT, + permission_keys: [AppACLPermission.AccessPoint], + } const result = getRedirectionPath(app) expect(result).toBe('/app/app-123/access-point') }) + it('returns apps list path when app ACL cannot access guarded pages or access point', () => { + const app = { id: 'app-123', mode: AppModeEnum.CHAT, permission_keys: [] } + const result = getRedirectionPath(app) + expect(result).toBe('/apps') + }) + it('returns workflow path for workflow mode when app ACL can access layout', () => { const app = { id: 'app-123', @@ -92,7 +102,11 @@ describe('app-redirection', () => { }) it('handles different app IDs', () => { - const app1 = { id: 'abc-123', mode: AppModeEnum.CHAT, permission_keys: [] } + const app1 = { + id: 'abc-123', + mode: AppModeEnum.CHAT, + permission_keys: [AppACLPermission.AccessPoint], + } const app2 = { id: 'xyz-789', mode: AppModeEnum.WORKFLOW, @@ -129,7 +143,7 @@ describe('app-redirection', () => { const app = { id: 'app-123', mode: AppModeEnum.CHAT, - permission_keys: [AppACLPermission.AccessConfig], + permission_keys: [AppACLPermission.AccessConfig, AppACLPermission.AccessPoint], } expect(getRedirectionPath(app, { isRbacEnabled: false })).toBe('/app/app-123/access-point') @@ -173,8 +187,12 @@ describe('app-redirection', () => { /** * Tests that the redirection function is called with the correct path */ - it('calls redirection function with access point path when app ACL cannot access guarded pages', () => { - const app = { id: 'app-123', mode: AppModeEnum.CHAT, permission_keys: [] } + it('calls redirection function with access point path when access point permission is granted', () => { + const app = { + id: 'app-123', + mode: AppModeEnum.CHAT, + permission_keys: [AppACLPermission.AccessPoint], + } const mockRedirect = vi.fn() getRedirection(app, mockRedirect) diff --git a/web/utils/app-redirection.ts b/web/utils/app-redirection.ts index 4bdfce9a266..d73a350b3f3 100644 --- a/web/utils/app-redirection.ts +++ b/web/utils/app-redirection.ts @@ -34,7 +34,9 @@ export const getRedirectionPath = ( if (app.mode === AppModeEnum.WORKFLOW && appACLCapabilities.canDeploy) return `/app/${app.id}/deploy` - return `/app/${app.id}/access-point` + if (appACLCapabilities.canAccessPoint) return `/app/${app.id}/access-point` + + return '/apps' } export const getRedirection = ( diff --git a/web/utils/permission.spec.ts b/web/utils/permission.spec.ts index 75dff3e7afe..487c9c891b1 100644 --- a/web/utils/permission.spec.ts +++ b/web/utils/permission.spec.ts @@ -50,6 +50,15 @@ describe('permission', () => { expect(releaseCapabilities.canDeploy).toBe(false) }) + it('keeps access point permission independent from other app ACL permissions', () => { + const accessPointCapabilities = getAppACLCapabilities([AppACLPermission.AccessPoint]) + const layoutCapabilities = getAppACLCapabilities([AppACLPermission.ViewLayout]) + + expect(accessPointCapabilities.canAccessPoint).toBe(true) + expect(accessPointCapabilities.canAccessLayout).toBe(false) + expect(layoutCapabilities.canAccessPoint).toBe(false) + }) + it('keeps monitor, tracing config, and log/annotation permissions independent', () => { const monitorCapabilities = getAppACLCapabilities([AppACLPermission.Monitor]) const tracingCapabilities = getAppACLCapabilities([AppACLPermission.TracingConfig]) @@ -109,6 +118,7 @@ describe('permission', () => { }) expect(capabilities.canViewLayout).toBe(true) + expect(capabilities.canAccessPoint).toBe(true) expect(capabilities.canTestAndRun).toBe(true) expect(capabilities.canEdit).toBe(true) expect(capabilities.canImportExportDSL).toBe(true) diff --git a/web/utils/permission.ts b/web/utils/permission.ts index 063878b6607..37b3a9e114b 100644 --- a/web/utils/permission.ts +++ b/web/utils/permission.ts @@ -2,6 +2,7 @@ import type { PermissionKey } from '@/models/access-control' export const AppACLPermission = { Preview: 'app.acl.preview', + AccessPoint: 'access_point.manage', ViewLayout: 'app.acl.view_layout', TestAndRun: 'app.acl.test_and_run', Edit: 'app.acl.edit', @@ -38,6 +39,7 @@ export type ResourceMaintainerPermissionOptions = { } type AppACLCapabilities = { + canAccessPoint: boolean canViewLayout: boolean canTestAndRun: boolean canEdit: boolean @@ -135,6 +137,11 @@ export const getAppACLCapabilities = ( ) return { + canAccessPoint: hasResourcePermission( + permissionKeys, + AppACLPermission.AccessPoint, + hasMaintainerPermissions, + ), canViewLayout, canTestAndRun, canEdit,