Merge pull request #2545 from trycompai/fix/cloud-reconnect-oauth-clear

[dev] [tofikwest] fix/cloud-reconnect-oauth-clear

Author: tofikwest

apps/api/src/cloud-security/providers/gcp-security.service.ts

@@ -245,22 +245,39 @@ export class GCPSecurityService {
     const { accessToken, organizationId, projectId } = params;
     const steps: GcpSetupStep[] = [];
     const email = await this.detectEmail(accessToken);
+    const hasFindingsAccess = organizationId
+      ? await this.canReadFindings(accessToken, organizationId)
+      : false;
 
     for (const stepDef of REQUIRED_GCP_API_STEPS) {
-      steps.push(
-        await this.runEnableApiSetupStep({
-          stepDef,
-          accessToken,
-          projectId,
-        }),
-      );
+      let step = await this.runEnableApiSetupStep({
+        stepDef,
+        accessToken,
+        projectId,
+      });
+
+      // If findings are already readable, SCC API access is effectively working for this org.
+      if (
+        stepDef.id === 'enable_security_command_center_api' &&
+        !step.success &&
+        hasFindingsAccess
+      ) {
+        step = {
+          ...step,
+          success: true,
+          error: undefined,
+        };
+      }
+
+      steps.push(step);
     }
 
     steps.push(
       await this.runGrantFindingsViewerSetupStep({
         accessToken,
         organizationId,
         email,
+        hasFindingsAccess,
       }),
     );
 
@@ -279,6 +296,9 @@ export class GCPSecurityService {
   }): Promise<{ email: string | null; step: GcpSetupStep }> {
     const { stepId, accessToken, organizationId, projectId } = params;
     const email = params.email ?? (await this.detectEmail(accessToken));
+    const hasFindingsAccess = organizationId
+      ? await this.canReadFindings(accessToken, organizationId)
+      : false;
 
     if (stepId === 'grant_findings_viewer_role') {
       return {
@@ -287,6 +307,7 @@ export class GCPSecurityService {
           accessToken,
           organizationId,
           email,
+          hasFindingsAccess,
         }),
       };
     }
@@ -296,14 +317,25 @@ export class GCPSecurityService {
       throw new Error(`Unsupported GCP setup step: ${stepId}`);
     }
 
-    return {
-      email,
-      step: await this.runEnableApiSetupStep({
-        stepDef,
-        accessToken,
-        projectId,
-      }),
-    };
+    let step = await this.runEnableApiSetupStep({
+      stepDef,
+      accessToken,
+      projectId,
+    });
+
+    if (
+      stepDef.id === 'enable_security_command_center_api' &&
+      !step.success &&
+      hasFindingsAccess
+    ) {
+      step = {
+        ...step,
+        success: true,
+        error: undefined,
+      };
+    }
+
+    return { email, step };
   }
 
   private async detectEmail(accessToken: string): Promise<string | null> {
@@ -408,8 +440,20 @@ export class GCPSecurityService {
     accessToken: string;
     organizationId: string;
     email: string | null;
+    hasFindingsAccess?: boolean;
   }): Promise<GcpSetupStep> {
-    const { accessToken, organizationId, email } = params;
+    const { accessToken, organizationId, email, hasFindingsAccess } = params;
+
+    // If we can already read findings, required scan permission exists.
+    // Don't fail setup just because this user cannot grant IAM roles.
+    if (hasFindingsAccess) {
+      return {
+        id: 'grant_findings_viewer_role',
+        name: 'Grant Findings Viewer role',
+        success: true,
+        ...FINDINGS_VIEWER_ACTION,
+      };
+    }
 
     if (!email) {
       return {
@@ -600,6 +644,30 @@ export class GCPSecurityService {
     }
   }
 
+  private async canReadFindings(
+    accessToken: string,
+    organizationId: string,
+  ): Promise<boolean> {
+    try {
+      const url = new URL(
+        `https://securitycenter.googleapis.com/v2/organizations/${organizationId}/sources/-/findings`,
+      );
+      url.searchParams.set('pageSize', '1');
+      url.searchParams.set('filter', 'state="ACTIVE"');
+
+      const response = await fetch(url.toString(), {
+        headers: {
+          Authorization: `Bearer ${accessToken}`,
+          'Content-Type': 'application/json',
+        },
+      });
+
+      return response.ok;
+    } catch {
+      return false;
+    }
+  }
+
   private buildFindingsViewerAdminActions(params: {
     organizationId: string;
     email: string;

apps/api/src/integration-platform/controllers/services.controller.ts

@@ -89,8 +89,15 @@ export class ServicesController {
           } else if (detectedServices) {
             enabled = detectedServices.includes(s.id) && !disabledServices.has(s.id);
           } else {
-            // Default: use enabledByDefault from manifest, otherwise enabled
-            enabled = (s.enabledByDefault ?? true) && !disabledServices.has(s.id);
+            // No detection data yet:
+            // - GCP should default to enabled (scan already runs across SCC categories by default),
+            //   so UI doesn't misleadingly show everything as OFF immediately after OAuth connect.
+            // - Others keep manifest defaults.
+            if (providerSlug === 'gcp') {
+              enabled = !disabledServices.has(s.id);
+            } else {
+              enabled = (s.enabledByDefault ?? true) && !disabledServices.has(s.id);
+            }
           }
 
           return {

apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.test.tsx

@@ -17,9 +17,23 @@ vi.mock('@/hooks/use-permissions', () => ({
 }));
 
 // Mock integration platform hooks
-const mockStartOAuth = vi.fn();
-const mockUseIntegrationProviders = vi.fn();
-const mockUseIntegrationConnections = vi.fn();
+const {
+  mockStartOAuth,
+  mockUseIntegrationProviders,
+  mockUseIntegrationConnections,
+  mockUseVendors,
+} = vi.hoisted(() => ({
+  mockStartOAuth: vi.fn(),
+  mockUseIntegrationProviders: vi.fn(),
+  mockUseIntegrationConnections: vi.fn(),
+  mockUseVendors: vi.fn(),
+}));
+
+const { mockRouterPush, mockUseSearchParams } = vi.hoisted(() => ({
+  mockRouterPush: vi.fn(),
+  mockUseSearchParams: vi.fn(() => new URLSearchParams()),
+}));
+
 vi.mock('@/hooks/use-integration-platform', () => ({
   useIntegrationProviders: mockUseIntegrationProviders,
   useIntegrationConnections: mockUseIntegrationConnections,
@@ -28,7 +42,6 @@ vi.mock('@/hooks/use-integration-platform', () => ({
   }),
 }));
 
-const mockUseVendors = vi.fn();
 vi.mock('@/hooks/use-vendors', () => ({
   useVendors: mockUseVendors,
 }));
@@ -78,8 +91,8 @@ vi.mock('next/link', () => ({
 // Mock next/navigation
 vi.mock('next/navigation', () => ({
   useParams: () => ({ orgId: 'org-1' }),
-  useRouter: () => ({ push: vi.fn() }),
-  useSearchParams: () => new URLSearchParams(),
+  useRouter: () => ({ push: mockRouterPush }),
+  useSearchParams: mockUseSearchParams,
 }));
 
 // Mock @trycompai/ui components
@@ -145,6 +158,7 @@ const defaultProps = {
 describe('PlatformIntegrations', () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    mockUseSearchParams.mockReturnValue(new URLSearchParams() as any);
     mockUseIntegrationProviders.mockReturnValue({
       providers: [
         {
@@ -221,6 +235,53 @@ describe('PlatformIntegrations', () => {
       expect(screen.getByText('GitHub')).toBeInTheDocument();
       expect(screen.getByTestId('search-input')).toBeInTheDocument();
     });
+
+    it('treats provider as connected when an active connection exists alongside older disconnected rows', () => {
+      setMockPermissions(ADMIN_PERMISSIONS);
+      mockUseIntegrationProviders.mockReturnValue({
+        providers: [
+          {
+            id: 'gcp',
+            name: 'Google Cloud Platform',
+            description: 'Cloud security',
+            category: 'Cloud',
+            logoUrl: '/gcp.png',
+            authType: 'oauth2',
+            oauthConfigured: true,
+            isActive: true,
+            requiredVariables: [],
+            mappedTasks: [],
+            supportsMultipleConnections: true,
+          },
+        ],
+        isLoading: false,
+      });
+      mockUseIntegrationConnections.mockReturnValue({
+        connections: [
+          // Newest row returned first by API
+          {
+            id: 'conn-new-active',
+            providerSlug: 'gcp',
+            status: 'active',
+            variables: {},
+            createdAt: '2026-04-14T00:00:00.000Z',
+          },
+          {
+            id: 'conn-old-disconnected',
+            providerSlug: 'gcp',
+            status: 'disconnected',
+            variables: {},
+            createdAt: '2026-04-01T00:00:00.000Z',
+          },
+        ] as any,
+        isLoading: false,
+        refresh: vi.fn(),
+      });
+
+      render(<PlatformIntegrations {...defaultProps} />);
+
+      expect(screen.queryByText('Connect')).not.toBeInTheDocument();
+    });
   });
 
   describe('Employee sync import prompt', () => {
@@ -266,10 +327,7 @@ describe('PlatformIntegrations', () => {
       });
 
       // Mock useSearchParams to simulate OAuth callback
-      const { useSearchParams: mockUseSearchParams } = vi.mocked(
-        await import('next/navigation'),
-      );
-      vi.mocked(mockUseSearchParams).mockReturnValue(
+      mockUseSearchParams.mockReturnValue(
         new URLSearchParams('success=true&provider=google-workspace') as any,
       );
 
@@ -330,10 +388,7 @@ describe('PlatformIntegrations', () => {
         refresh: vi.fn(),
       });
 
-      const { useSearchParams: mockUseSearchParams } = vi.mocked(
-        await import('next/navigation'),
-      );
-      vi.mocked(mockUseSearchParams).mockReturnValue(
+      mockUseSearchParams.mockReturnValue(
         new URLSearchParams('success=true&provider=github') as any,
       );
 

apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.tsx

@@ -94,6 +94,38 @@ interface PlatformIntegrationsProps {
   taskTemplates: Array<{ id: string; taskId: string; name: string; description: string }>;
 }
 
+const CONNECTION_STATUS_PRIORITY: Record<string, number> = {
+  active: 5,
+  pending: 4,
+  error: 3,
+  paused: 2,
+  disconnected: 1,
+};
+
+const getConnectionPriority = (connection: ConnectionListItem): number => {
+  return CONNECTION_STATUS_PRIORITY[connection.status] ?? 0;
+};
+
+const getConnectionCreatedAtMs = (connection: ConnectionListItem): number => {
+  const date = new Date(connection.createdAt);
+  return Number.isNaN(date.getTime()) ? 0 : date.getTime();
+};
+
+const shouldReplaceProviderConnection = (
+  current: ConnectionListItem | undefined,
+  candidate: ConnectionListItem,
+): boolean => {
+  if (!current) return true;
+
+  const currentPriority = getConnectionPriority(current);
+  const candidatePriority = getConnectionPriority(candidate);
+  if (candidatePriority !== currentPriority) {
+    return candidatePriority > currentPriority;
+  }
+
+  return getConnectionCreatedAtMs(candidate) > getConnectionCreatedAtMs(current);
+};
+
 export function PlatformIntegrations({ className, taskTemplates }: PlatformIntegrationsProps) {
   const { orgId } = useParams<{ orgId: string }>();
   const router = useRouter();
@@ -190,10 +222,16 @@ export function PlatformIntegrations({ className, taskTemplates }: PlatformInteg
   };
 
   // Map connections by provider slug
-  const connectionsByProvider = useMemo(
-    () => new Map(connections?.map((c) => [c.providerSlug, c]) || []),
-    [connections],
-  );
+  const connectionsByProvider = useMemo(() => {
+    const map = new Map<string, ConnectionListItem>();
+    for (const connection of connections ?? []) {
+      const current = map.get(connection.providerSlug);
+      if (shouldReplaceProviderConnection(current, connection)) {
+        map.set(connection.providerSlug, connection);
+      }
+    }
+    return map;
+  }, [connections]);
 
   const vendorNames = useMemo(() => {
     const vendors = vendorsResponse?.data?.data;