From b9ab634cd0d22357dbc395eba097c5ce026aa46d Mon Sep 17 00:00:00 2001 From: Alaister Young Date: Mon, 27 Jul 2026 17:58:32 +0800 Subject: [PATCH] fix(studio): stop 403'd integration queries from looping on remount (#48350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves FE-4014 A user with a project-scoped role opening any project integration overview (e.g. Cron) hits an unbounded request loop — the page sits on a skeleton forever while hammering the platform API until it gets rate limited. **Changed:** - `useProjectOAuthIntegrationData` now passes `retryOnMount: false` to its five queries, so a 403 settles as a terminal error instead of refetching on every consumer mount ## Why Project-scoped roles have no org-level permissions, so `GET /platform/organizations/{slug}/oauth/apps` 403s. We don't retry 4xx, so the query settles into `error` with no data — and an errored query with no data is never fresh, so it refetches on *every* new observer mount. That feeds a loop: refetch → `isLoading` true → `IntegrationPage` swaps its whole subtree to a skeleton → `` unmounts → 403 lands → `isLoading` false → remounts → mounts fresh observers → refetch. Measured ~20 req/s (480 observer add/removes and 120 requests in a 6s window) until the API 429s it, then it continues at the retry cadence indefinitely. The other four queries in that hook can 403 the same way for restricted roles, and any one of them alone sustains the loop — hence the option on all five. Not fixed here: `IntegrationPage` tearing down its subtree whenever `isLoading` flips (`pages/project/[ref]/integrations/[id]/[pageId]/[childId]/index.tsx:58-94`) is the amplifier that turns a wasted request into a loop, and will still reset UI state on any background refetch. Worth a follow-up. ## To test Needs an account with a project-scoped role in a shared org (not an org owner/admin). - Open `/project/{ref}/integrations` for that project, click into Cron (or any integration) → overview should render, not sit on a skeleton - Network tab: `organizations/{slug}/oauth/apps?type=authorized` should fire once and 403, not repeat - Console should show 1 error, not hundreds ending in a 429 - As an org owner, integration overviews should behave exactly as before ## Summary by CodeRabbit * **Bug Fixes** * Prevented repeated refetching of integration data after handled authorization/403 errors, avoiding refetch loops on remount. * Improved consistency on integration landing screens by standardizing how related integration queries are enabled and retried. * **Enhancements** * Added permission-aware loading/error handling for OAuth integration data, showing OAuth results only when the selected organization grants read access. * **Chores** * Updated permission-check typings to treat an explicitly empty project reference as absent. * **Tests** * Extended integration settings tests with permission fixtures to cover OAuth read access. --------- Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com> Co-authored-by: Joshen Lim --- ...MarketplaceIntegrationSettingsTab.test.tsx | 26 +++++++++++++++ .../Integrations/Landing/Landing.utils.ts | 32 ++++++++++++++++--- .../Landing/useInstalledIntegrations.tsx | 22 ++++++++++--- apps/studio/hooks/misc/useCheckPermissions.ts | 13 +++++--- 4 files changed, 80 insertions(+), 13 deletions(-) diff --git a/apps/studio/components/interfaces/Integrations/Integration/MarketplaceIntegrationSettingsTab.test.tsx b/apps/studio/components/interfaces/Integrations/Integration/MarketplaceIntegrationSettingsTab.test.tsx index ba0721575c0..56cab656c94 100644 --- a/apps/studio/components/interfaces/Integrations/Integration/MarketplaceIntegrationSettingsTab.test.tsx +++ b/apps/studio/components/interfaces/Integrations/Integration/MarketplaceIntegrationSettingsTab.test.tsx @@ -1,3 +1,4 @@ +import { PermissionAction } from '@supabase/shared-types/out/constants' import { fireEvent, screen, waitFor } from '@testing-library/react' import { mockAnimationsApi } from 'jsdom-testing-mocks' import { HttpResponse } from 'msw' @@ -16,6 +17,9 @@ import { addAPIMock } from '@/tests/lib/msw' // the MSW resolver) is the raw OpenAPI `OAuthAppResponse`. Build fixtures against the API shape. type OAuthAppResponse = components['schemas']['OAuthAppResponse'] type PartnerIntegrationListResponse = components['schemas']['PartnerIntegrationListResponse'] +// Same story for permissions: the frontend `Permission` type narrows nullable wire fields, so build +// the fixture against the raw `AccessControlPermission` shape the MSW resolver actually returns. +type AccessControlPermission = components['schemas']['AccessControlPermission'] // `useIntegrationDetail` resolves the integration definition from the route, the marketplace query // and feature flags — none of which is the subject of this test. Mock it so each test can drive a @@ -31,6 +35,12 @@ vi.mock('@/components/interfaces/Integrations/Landing/useIntegrationDetail', () vi.mock('@/hooks/misc/useSelectedOrganization', () => ({ useSelectedOrganizationQuery: () => ({ data: { slug: 'acme' } }), })) +// `useProjectOAuthIntegrationData` gates the authorized-apps query behind a permission check, which +// requires a logged-in user and a granted `oauth_apps` permission to ever fire. +vi.mock('common', async (importOriginal) => { + const actual = await importOriginal>() + return { ...actual, useIsLoggedIn: () => true } +}) // The auth-config query (used to detect custom SMTP) only runs on the platform. vi.mock('@/lib/constants', async (importOriginal) => { const actual = await importOriginal>() @@ -74,6 +84,17 @@ const authorizedApp = (appId: string): OAuthAppResponse => ({ registration_type: 'manual', }) +const CAN_READ_OAUTH_APPS_PERMISSION: AccessControlPermission = { + actions: [PermissionAction.READ], + condition: null, + organization_id: null, + organization_slug: 'acme', + project_ids: null, + project_refs: [], + resources: ['oauth_apps'], + restrictive: false, +} + /** * Registers the five endpoints behind `useProjectOAuthIntegrationData`. Each test supplies only the * resources relevant to it; everything else defaults to "nothing connected". @@ -84,6 +105,11 @@ const mockProjectResources = ({ oauthApps = [] as OAuthAppResponse[], smtpHost = null as string | null, } = {}) => { + addAPIMock({ + method: 'get', + path: '/platform/profile/permissions', + response: () => HttpResponse.json([CAN_READ_OAUTH_APPS_PERMISSION]), + }) addAPIMock({ method: 'get', path: '/v1/projects/:ref/api-keys', diff --git a/apps/studio/components/interfaces/Integrations/Landing/Landing.utils.ts b/apps/studio/components/interfaces/Integrations/Landing/Landing.utils.ts index 9315eef4ba0..f524a3ba675 100644 --- a/apps/studio/components/interfaces/Integrations/Landing/Landing.utils.ts +++ b/apps/studio/components/interfaces/Integrations/Landing/Landing.utils.ts @@ -1,4 +1,5 @@ import { parseSchemaComment } from '@stripe/sync-engine/supabase' +import { PermissionAction } from '@supabase/shared-types/out/constants' import { useMemo } from 'react' import { type WrapperMeta } from '../Wrappers/Wrappers.types' @@ -19,6 +20,7 @@ import { usePartnerIntegrationsQuery, } from '@/data/partners/integration-status-query' import { useSecretsQuery, type ProjectSecret } from '@/data/secrets/secrets-query' +import { useAsyncCheckPermissions } from '@/hooks/misc/useCheckPermissions' import { useSelectedOrganizationQuery } from '@/hooks/misc/useSelectedOrganization' import { ResponseError } from '@/types' @@ -67,12 +69,32 @@ export const useProjectOAuthIntegrationData = ( isSuccess: boolean } => { const { data: org } = useSelectedOrganizationQuery({ enabled }) + + // Any error here has to be terminal on mount. An errored query holds no data, so it never counts + // as fresh and refetches on every consumer mount — and consumers that gate rendering on + // `isLoading` remount on each refetch, which loops. Transient failures still recover: the retry + // policy gives 5xx three attempts, and refetch-on-focus/reconnect are staleness-driven, so they + // are unaffected by this. + const { can: canReadOAuthApps } = useAsyncCheckPermissions( + PermissionAction.READ, + 'oauth_apps', + undefined, + { + organizationSlug: org?.slug, + projectRef: null, + } + ) + + const sharedOptions = { enabled, retryOnMount: false } const queries = { - apiKeys: useAPIKeysQuery({ projectRef, reveal: false }, { enabled }), - edgeFunctionSecrets: useSecretsQuery({ projectRef }, { enabled }), - authConfig: useAuthConfigQuery({ projectRef }, { enabled }), - partnerIntegrations: usePartnerIntegrationsQuery({ projectRef }, { enabled }), - oauthApps: useAuthorizedAppsQuery({ slug: org?.slug }, { enabled: enabled && !!org }), + apiKeys: useAPIKeysQuery({ projectRef, reveal: false }, sharedOptions), + edgeFunctionSecrets: useSecretsQuery({ projectRef }, sharedOptions), + authConfig: useAuthConfigQuery({ projectRef }, sharedOptions), + partnerIntegrations: usePartnerIntegrationsQuery({ projectRef }, sharedOptions), + oauthApps: useAuthorizedAppsQuery( + { slug: org?.slug }, + { ...sharedOptions, enabled: canReadOAuthApps && enabled && !!org } + ), } const data = useMemo(() => { diff --git a/apps/studio/components/interfaces/Integrations/Landing/useInstalledIntegrations.tsx b/apps/studio/components/interfaces/Integrations/Landing/useInstalledIntegrations.tsx index 32359359859..2f6ec849849 100644 --- a/apps/studio/components/interfaces/Integrations/Landing/useInstalledIntegrations.tsx +++ b/apps/studio/components/interfaces/Integrations/Landing/useInstalledIntegrations.tsx @@ -1,3 +1,4 @@ +import { PermissionAction } from '@supabase/shared-types/out/constants' import { useMemo } from 'react' import { @@ -11,11 +12,24 @@ import { useAvailableIntegrations } from './useAvailableIntegrations' import { useDatabaseExtensionsQuery } from '@/data/database-extensions/database-extensions-query' import { useSchemasQuery } from '@/data/database/schemas-query' import { useFDWsQuery } from '@/data/fdw/fdws-query' +import { useAsyncCheckPermissions } from '@/hooks/misc/useCheckPermissions' +import { useSelectedOrganizationQuery } from '@/hooks/misc/useSelectedOrganization' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' import { EMPTY_ARR } from '@/lib/void' export const useInstalledIntegrations = () => { const { data: project } = useSelectedProjectQuery() + const { data: org } = useSelectedOrganizationQuery() + + const { can: canReadOAuthApps } = useAsyncCheckPermissions( + PermissionAction.READ, + 'oauth_apps', + undefined, + { + organizationSlug: org?.slug, + projectRef: null, + } + ) const { data: allIntegrations = EMPTY_ARR, @@ -101,25 +115,25 @@ export const useInstalledIntegrations = () => { extensionsError || schemasError || availableIntegrationsError || - (hasOAuthIntegration ? oauthDataError : null) + (canReadOAuthApps && hasOAuthIntegration ? oauthDataError : null) const isLoading = isSchemasLoading || isFDWLoading || isExtensionsLoading || isAvailableIntegrationsLoading || - (hasOAuthIntegration && isOAuthDataLoading) + (hasOAuthIntegration && canReadOAuthApps && isOAuthDataLoading) const isError = isErrorFDWs || isErrorExtensions || isErrorSchemas || isErrorAvailableIntegrations || - (hasOAuthIntegration && isErrorOAuthData) + (hasOAuthIntegration && canReadOAuthApps && isErrorOAuthData) const isSuccess = isSuccessFDWs && isSuccessExtensions && isSuccessSchemas && isSuccessAvailableIntegrations && - (!hasOAuthIntegration || isSuccessOAuthData) + (!hasOAuthIntegration || !canReadOAuthApps || isSuccessOAuthData) return { // show all integrations at once instead of showing partial results diff --git a/apps/studio/hooks/misc/useCheckPermissions.ts b/apps/studio/hooks/misc/useCheckPermissions.ts index 5d77713a96a..68868edae3c 100644 --- a/apps/studio/hooks/misc/useCheckPermissions.ts +++ b/apps/studio/hooks/misc/useCheckPermissions.ts @@ -34,7 +34,7 @@ export function doPermissionsCheck( resource: string, data?: object, organizationSlug?: string, - projectRef?: string + projectRef?: string | null ) { if (!permissions || !Array.isArray(permissions)) { return false @@ -76,7 +76,7 @@ export function useGetPermissions( function useGetProjectPermissions( permissionsOverride?: Permission[], organizationSlugOverride?: string, - projectRefOverride?: string, + projectRefOverride?: string | null, enabled = true ) { const { @@ -114,7 +114,12 @@ function useGetProjectPermissions( ? projectData : { ref: projectRefOverride, parent_project_ref: undefined } - const projectRef = project?.parent_project_ref ? project.parent_project_ref : project?.ref + const projectRef = + projectRefOverride === null + ? null + : project?.parent_project_ref + ? project.parent_project_ref + : project?.ref const isLoading = isLoadingPermissions || @@ -142,7 +147,7 @@ export function useAsyncCheckPermissions( data?: object, overrides?: { organizationSlug?: string - projectRef?: string + projectRef?: string | null permissions?: Permission[] } ) {