diff --git a/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.test.ts b/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.test.ts new file mode 100644 index 000000000..f866b8f95 --- /dev/null +++ b/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.test.ts @@ -0,0 +1,124 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { useEnvScopedPermission } from './useEnvScopedPermission'; + +const mockUsePermission = jest.fn(); +jest.mock('@backstage/plugin-permission-react', () => ({ + usePermission: (...args: any[]) => mockUsePermission(...args), +})); + +const mockGetBaseUrl = jest.fn(); +const mockFetch = jest.fn(); +// Stable API singletons so useApi returns the same reference each render — +// otherwise the env-eval useEffect (which has `discovery` and `fetchApi` in +// its dep array) re-runs on every render and spirals into an infinite loop. +const mockDiscoveryApi = { getBaseUrl: mockGetBaseUrl }; +const mockFetchApi = { fetch: mockFetch }; +jest.mock('@backstage/core-plugin-api', () => { + const discoveryRef = Symbol('discoveryApiRef'); + const fetchRef = Symbol('fetchApiRef'); + return { + discoveryApiRef: discoveryRef, + fetchApiRef: fetchRef, + useApi: (apiRef: symbol) => { + if (apiRef === discoveryRef) return mockDiscoveryApi; + if (apiRef === fetchRef) return mockFetchApi; + return {}; + }, + }; +}); + +const mockUseAuthzEnabled = jest.fn(); +jest.mock('./useOpenChoreoFeatures', () => ({ + useAuthzEnabled: () => mockUseAuthzEnabled(), +})); + +// Minimal Permission stand-in. Avoids depending on +// @backstage/plugin-permission-common in this package's devDeps; the hook +// only reads `permission.name` and a couple of type-narrowing properties. +const basicPermission = { + type: 'basic' as const, + name: 'openchoreo.releasebinding.view', + attributes: { action: 'read' as const }, +}; + +function renderEnvHook(environment: string | undefined) { + return renderHook(() => + useEnvScopedPermission({ + permission: basicPermission, + resourceRef: 'component:team-shop/snip-api-service', + environment, + }), + ); +} + +beforeEach(() => { + jest.clearAllMocks(); + mockGetBaseUrl.mockResolvedValue('http://localhost/api/permission'); + mockUseAuthzEnabled.mockReturnValue(true); +}); + +describe('useEnvScopedPermission', () => { + it('returns env-eval ALLOW when authz on, base allowed, backend allows', async () => { + mockUsePermission.mockReturnValue({ allowed: true, loading: false }); + mockFetch.mockResolvedValue({ + ok: true, + json: async () => ({ allowed: true }), + }); + + const { result } = renderEnvHook('development'); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.allowed).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('returns env-eval DENY when authz on, base allowed, backend denies', async () => { + mockUsePermission.mockReturnValue({ allowed: true, loading: false }); + mockFetch.mockResolvedValue({ + ok: true, + json: async () => ({ allowed: false }), + }); + + const { result } = renderEnvHook('production'); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.allowed).toBe(false); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + // Regression test for the bug fixed alongside this test file: when + // openchoreo.features.authz.enabled = false the policy backend installs + // AllowAllPolicy and does NOT mount /evaluate-with-context. Firing the + // fetch would 404 and fail closed to "denied" on every env tile. + it('does NOT fetch /evaluate-with-context and propagates baseCheck.allowed when authz disabled', async () => { + mockUseAuthzEnabled.mockReturnValue(false); + mockUsePermission.mockReturnValue({ allowed: true, loading: false }); + + const { result } = renderEnvHook('development'); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.allowed).toBe(true); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('still propagates baseCheck.allowed=false when authz disabled (degrade to base only)', async () => { + mockUseAuthzEnabled.mockReturnValue(false); + mockUsePermission.mockReturnValue({ allowed: false, loading: false }); + + const { result } = renderEnvHook('development'); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.allowed).toBe(false); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('bypasses env-eval when no environment supplied, regardless of authzEnabled', async () => { + mockUsePermission.mockReturnValue({ allowed: true, loading: false }); + + const { result } = renderEnvHook(undefined); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.allowed).toBe(true); + expect(mockFetch).not.toHaveBeenCalled(); + }); +}); diff --git a/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.ts b/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.ts index cb2c9ac98..51f2e4a01 100644 --- a/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.ts +++ b/plugins/openchoreo-react/src/hooks/useEnvScopedPermission.ts @@ -9,6 +9,7 @@ import type { Permission, ResourcePermission, } from '@backstage/plugin-permission-common'; +import { useAuthzEnabled } from './useOpenChoreoFeatures'; interface UseEnvScopedPermissionOptions { /** Backstage permission to evaluate (must map to an OpenChoreo action). */ @@ -44,6 +45,17 @@ interface UseEnvScopedPermissionResult { * * The combined `allowed` is the AND of both checks: visibility must pass * AND (no environment specified OR env-specific evaluation passed). + * + * **Disabled-authz path.** When `openchoreo.features.authz.enabled = false`, + * the policy module installs `AllowAllPolicy` and does NOT mount the + * `/evaluate-with-context` route (see + * `permission-backend-module-openchoreo-policy/src/module.ts`). Firing the + * fetch anyway would 404 and fail closed to "denied" on every env tile. + * Degrade gracefully: skip the env-eval entirely and return whatever + * `usePermission` reports (which is `allowed: true` under AllowAllPolicy + * for OpenChoreo resource permissions). This keeps the chokepoint in one + * place so every consumer hook (`useDeployPermission`, …) inherits the + * correct behavior without each repeating the guard. */ export function useEnvScopedPermission( options: UseEnvScopedPermissionOptions, @@ -58,6 +70,8 @@ export function useEnvScopedPermission( : ({ permission, resourceRef } as Parameters[0]), ); + const authzEnabled = useAuthzEnabled(); + const discovery = useApi(discoveryApiRef); const fetchApi = useApi(fetchApiRef); @@ -65,6 +79,15 @@ export function useEnvScopedPermission( const [envLoading, setEnvLoading] = useState(false); useEffect(() => { + // Authz disabled → backend policy is AllowAllPolicy and the + // /evaluate-with-context route is not mounted. Skip the fetch. + // The final return below degrades to `baseCheck` so we propagate + // whatever AllowAllPolicy decided. + if (!authzEnabled) { + setEnvAllowed(undefined); + setEnvLoading(false); + return undefined; + } // No environment supplied → skip env-specific check. Clear envLoading // explicitly so a previous cancelled fetch doesn't leave it stuck true. if (!environment) { @@ -123,11 +146,12 @@ export function useEnvScopedPermission( permission.name, resourceRef, environment, + authzEnabled, baseCheck.allowed, baseCheck.loading, ]); - if (!environment) { + if (!authzEnabled || !environment) { return { allowed: baseCheck.allowed, loading: baseCheck.loading }; } const loading = baseCheck.loading || envLoading || envAllowed === undefined;