Skip to content

Commit 4ae0b3d

Browse files
authored
[FE-1722] fix: race condition in permissions hooks (supabase#37288)
fix: race condition in permissions hooks
1 parent de509e2 commit 4ae0b3d

File tree

3 files changed

+138
-17
lines changed

3 files changed

+138
-17
lines changed

apps/studio/hooks/misc/useCheckPermissions.ts

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import { usePermissionsQuery } from 'data/permissions/permissions-query'
66
import { useProjectDetailQuery } from 'data/projects/project-detail-query'
77
import { IS_PLATFORM } from 'lib/constants'
88
import type { Permission } from 'types'
9-
import { useSelectedOrganization } from './useSelectedOrganization'
10-
import { useSelectedProject } from './useSelectedProject'
9+
import { useSelectedOrganizationQuery } from './useSelectedOrganization'
10+
import { useSelectedProjectQuery } from './useSelectedProject'
1111

1212
const toRegexpString = (actionOrResource: string) =>
1313
`^${actionOrResource.replace('.', '\\.').replace('%', '.*')}$`
@@ -80,30 +80,50 @@ export function useGetProjectPermissions(
8080
projectRefOverride?: string,
8181
enabled = true
8282
) {
83-
const { data, isLoading, isSuccess } = usePermissionsQuery({
83+
const {
84+
data,
85+
isLoading: isLoadingPermissions,
86+
isSuccess: isSuccessPermissions,
87+
} = usePermissionsQuery({
8488
enabled: permissionsOverride === undefined && enabled,
8589
})
86-
8790
const permissions = permissionsOverride === undefined ? data : permissionsOverride
8891

89-
const organizationResult = useSelectedOrganization({
90-
enabled: organizationSlugOverride === undefined && enabled,
92+
const organizationsQueryEnabled = organizationSlugOverride === undefined && enabled
93+
const {
94+
data: organizationData,
95+
isLoading: isLoadingOrganization,
96+
isSuccess: isSuccessOrganization,
97+
} = useSelectedOrganizationQuery({
98+
enabled: organizationsQueryEnabled,
9199
})
92-
93100
const organization =
94-
organizationSlugOverride === undefined ? organizationResult : { slug: organizationSlugOverride }
101+
organizationSlugOverride === undefined ? organizationData : { slug: organizationSlugOverride }
95102
const organizationSlug = organization?.slug
96103

97-
const projectResult = useSelectedProject({
98-
enabled: projectRefOverride === undefined && enabled,
104+
const projectsQueryEnabled = projectRefOverride === undefined && enabled
105+
const {
106+
data: projectData,
107+
isLoading: isLoadingProject,
108+
isSuccess: isSuccessProject,
109+
} = useSelectedProjectQuery({
110+
enabled: projectsQueryEnabled,
99111
})
100-
101112
const project =
102-
projectRefOverride === undefined || projectResult?.parent_project_ref
103-
? projectResult
113+
projectRefOverride === undefined || projectData?.parent_project_ref
114+
? projectData
104115
: { ref: projectRefOverride, parent_project_ref: undefined }
105116
const projectRef = project?.parent_project_ref ? project.parent_project_ref : project?.ref
106117

118+
const isLoading =
119+
isLoadingPermissions ||
120+
(organizationsQueryEnabled && isLoadingOrganization) ||
121+
(projectsQueryEnabled && isLoadingProject)
122+
const isSuccess =
123+
isSuccessPermissions &&
124+
(!organizationsQueryEnabled || isSuccessOrganization) &&
125+
(!projectsQueryEnabled || isSuccessProject)
126+
107127
return {
108128
permissions,
109129
organizationSlug,
@@ -196,22 +216,33 @@ export function useAsyncCheckProjectPermissions(
196216
isSuccess: isPermissionsSuccess,
197217
} = useGetProjectPermissions(permissions, organizationSlug, projectRef, isLoggedIn)
198218

199-
if (!isLoggedIn)
219+
if (!isLoggedIn) {
200220
return {
201221
isLoading: true,
202222
isSuccess: false,
203223
can: false,
204224
}
205-
if (!IS_PLATFORM)
225+
}
226+
if (!IS_PLATFORM) {
206227
return {
207228
isLoading: false,
208229
isSuccess: true,
209230
can: true,
210231
}
232+
}
233+
234+
const can = doPermissionsCheck(
235+
allPermissions,
236+
action,
237+
resource,
238+
data,
239+
_organizationSlug,
240+
_projectRef
241+
)
211242

212243
return {
213244
isLoading: isPermissionsLoading,
214245
isSuccess: isPermissionsSuccess,
215-
can: doPermissionsCheck(allPermissions, action, resource, data, _organizationSlug, _projectRef),
246+
can,
216247
}
217248
}

apps/studio/hooks/misc/useSelectedOrganization.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,20 @@ import { useIsLoggedIn, useParams } from 'common'
22
import { useOrganizationsQuery } from 'data/organizations/organizations-query'
33
import { useMemo } from 'react'
44

5-
import { useProjectByRef } from './useSelectedProject'
5+
import { useProjectByRef, useProjectByRefQuery } from './useSelectedProject'
66

7+
/**
8+
* @deprecated Use useSelectedOrganizationQuery instead for access to loading states etc
9+
*
10+
* Example migration:
11+
* ```
12+
* // Old:
13+
* const organization = useSelectedOrganization(ref)
14+
*
15+
* // New:
16+
* const { data: organization } = useSelectedOrganizationQuery(ref)
17+
* ```
18+
*/
719
export function useSelectedOrganization({ enabled = true } = {}) {
820
const isLoggedIn = useIsLoggedIn()
921

@@ -20,3 +32,21 @@ export function useSelectedOrganization({ enabled = true } = {}) {
2032
})
2133
}, [data, selectedProject, slug])
2234
}
35+
36+
export function useSelectedOrganizationQuery({ enabled = true } = {}) {
37+
const isLoggedIn = useIsLoggedIn()
38+
39+
const { ref, slug } = useParams()
40+
const { data: selectedProject } = useProjectByRefQuery(ref)
41+
42+
return useOrganizationsQuery({
43+
enabled: isLoggedIn && enabled,
44+
select: (data) => {
45+
return data.find((org) => {
46+
if (slug !== undefined) return org.slug === slug
47+
if (selectedProject !== undefined) return org.id === selectedProject.organization_id
48+
return undefined
49+
})
50+
},
51+
})
52+
}

apps/studio/hooks/misc/useSelectedProject.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@ import { useProjectDetailQuery } from 'data/projects/project-detail-query'
55
import { ProjectInfo, useProjectsQuery } from 'data/projects/projects-query'
66
import { PROVIDERS } from 'lib/constants'
77

8+
/**
9+
* @deprecated Use useSelectedProjectQuery instead for access to loading states etc
10+
*
11+
* Example migration:
12+
* ```
13+
* // Old:
14+
* const project = useSelectedProject()
15+
*
16+
* // New:
17+
* const { data: project } = useSelectedProjectQuery()
18+
* ```
19+
*/
820
export function useSelectedProject({ enabled = true } = {}) {
921
const { ref } = useParams()
1022
const { data } = useProjectDetailQuery({ ref }, { enabled })
@@ -15,6 +27,32 @@ export function useSelectedProject({ enabled = true } = {}) {
1527
)
1628
}
1729

30+
export function useSelectedProjectQuery({ enabled = true } = {}) {
31+
const { ref } = useParams()
32+
33+
return useProjectDetailQuery(
34+
{ ref },
35+
{
36+
enabled,
37+
select: (data) => {
38+
return { ...data, parentRef: data.parent_project_ref ?? data.ref }
39+
},
40+
}
41+
)
42+
}
43+
44+
/**
45+
* @deprecated Use useProjectByRefQuery instead for access to loading states etc
46+
*
47+
* Example migration:
48+
* ```
49+
* // Old:
50+
* const project = useProjectByRef(ref)
51+
*
52+
* // New:
53+
* const { data: project } = useProjectByRefQuery(ref)
54+
* ```
55+
*/
1856
export function useProjectByRef(
1957
ref?: string
2058
): Omit<ProjectInfo, 'organization_slug' | 'preview_branch_refs'> | undefined {
@@ -34,6 +72,28 @@ export function useProjectByRef(
3472
}, [project, projects, ref])
3573
}
3674

75+
export function useProjectByRefQuery(ref?: string) {
76+
const isLoggedIn = useIsLoggedIn()
77+
78+
const projectQuery = useProjectDetailQuery({ ref }, { enabled: isLoggedIn })
79+
80+
// [Alaister]: This is here for the purpose of improving performance.
81+
// Chances are, the user will already have the list of projects in the cache.
82+
// We can't exclusively rely on this method, as useProjectsQuery does not return branch projects.
83+
const projectsQuery = useProjectsQuery({
84+
enabled: isLoggedIn,
85+
select: (data) => {
86+
return data.find((project) => project.ref === ref)
87+
},
88+
})
89+
90+
if (projectQuery.isSuccess) {
91+
return projectQuery
92+
}
93+
94+
return projectsQuery
95+
}
96+
3797
export const useIsAwsCloudProvider = () => {
3898
const project = useSelectedProject()
3999
const isAws = project?.cloud_provider === PROVIDERS.AWS.id

0 commit comments

Comments
 (0)