From fe275460f4a1d90ee8847bdc9d2de793dc2056dd Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Mon, 13 Apr 2026 20:34:12 -0400 Subject: [PATCH 1/2] fix(gcp): capability-based setup checks and stable provider connection pick --- .../providers/gcp-security.service.ts | 100 +++++++++++++++--- .../components/PlatformIntegrations.test.tsx | 83 ++++++++++++--- .../components/PlatformIntegrations.tsx | 46 +++++++- 3 files changed, 195 insertions(+), 34 deletions(-) diff --git a/apps/api/src/cloud-security/providers/gcp-security.service.ts b/apps/api/src/cloud-security/providers/gcp-security.service.ts index 2b0a95715..01668f78b 100644 --- a/apps/api/src/cloud-security/providers/gcp-security.service.ts +++ b/apps/api/src/cloud-security/providers/gcp-security.service.ts @@ -245,15 +245,31 @@ 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( @@ -261,6 +277,7 @@ export class GCPSecurityService { 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 { @@ -408,8 +440,20 @@ export class GCPSecurityService { accessToken: string; organizationId: string; email: string | null; + hasFindingsAccess?: boolean; }): Promise { - 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 { + 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; diff --git a/apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.test.tsx b/apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.test.tsx index 24b374f89..c90a4b832 100644 --- a/apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.test.tsx +++ b/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(); + + 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, ); diff --git a/apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.tsx b/apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.tsx index e298e1b91..77a742105 100644 --- a/apps/app/src/app/(app)/[orgId]/integrations/components/PlatformIntegrations.tsx +++ b/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 = { + 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(); + 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; From 39e38b0e39c0171bc1b545e9fcf7f65f752182a7 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Mon, 13 Apr 2026 20:35:26 -0400 Subject: [PATCH 2/2] fix(integrations): default gcp services to enabled before detection data --- .../controllers/services.controller.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/api/src/integration-platform/controllers/services.controller.ts b/apps/api/src/integration-platform/controllers/services.controller.ts index d0e714b1d..427aba61b 100644 --- a/apps/api/src/integration-platform/controllers/services.controller.ts +++ b/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 {