fix(authz): skip env-scoped /evaluate-with-context when authz is disabled

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();
+  });
+});

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,13 +70,24 @@ export function useEnvScopedPermission(
       : ({ permission, resourceRef } as Parameters<typeof usePermission>[0]),
   );
 
+  const authzEnabled = useAuthzEnabled();
+
   const discovery = useApi(discoveryApiRef);
   const fetchApi = useApi(fetchApiRef);
 
   const [envAllowed, setEnvAllowed] = useState<boolean | undefined>(undefined);
   const [envLoading, setEnvLoading] = useState<boolean>(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;