mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 17:35:10 +03:00
fix(studio): stop 403'd integration queries from looping on remount (#48350)
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 → `<Component />` 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
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
Co-authored-by: Joshen Lim <joshenlimek@gmail.com>
This commit is contained in:
4 files changed
+80
-13
No files matched your search
+26
@@ -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<Record<string, unknown>>()
|
||||
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<Record<string, unknown>>()
|
||||
@@ -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<AccessControlPermission[]>([CAN_READ_OAUTH_APPS_PERMISSION]),
|
||||
})
|
||||
addAPIMock({
|
||||
method: 'get',
|
||||
path: '/v1/projects/:ref/api-keys',
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
+18
-4
@@ -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
|
||||
|
||||
@@ -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[]
|
||||
}
|
||||
) {
|
||||
|
||||
Reference in new issue
Block a user