Skip to content

Commit

Permalink
fix: prevent excessive requests to access endpoint (#7597)
Browse files Browse the repository at this point in the history
  • Loading branch information
rexxars authored Oct 15, 2024
1 parent 1504511 commit 45a74fc
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function getPairPermissions({
draft,
published,
liveEdit,
}: PairPermissionsOptions): Array<[string, Observable<PermissionCheckResult>] | null> {
}: PairPermissionsOptions): Array<[string, Observable<PermissionCheckResult>]> {
// this was introduced because we ran into a bug where a user with publish
// access was marked as not allowed to duplicate a document unless it had a
// draft variant. this would happen in non-live edit cases where the document
Expand Down Expand Up @@ -220,9 +220,9 @@ export function getDocumentPairPermissions({
draft,
published,
liveEdit,
}).map(([label, observable]: any) =>
observable.pipe(
map(({granted, reason}: any) => ({
}).map(([label, permissionCheck$]) =>
permissionCheck$.pipe(
map(({granted, reason}) => ({
granted,
reason: granted ? '' : `not allowed to ${label}: ${reason}`,
label,
Expand All @@ -234,7 +234,7 @@ export function getDocumentPairPermissions({
if (!pairPermissions.length) return of({granted: true, reason: ''})

return combineLatest(pairPermissions).pipe(
map((permissionResults: any[]) => {
map((permissionResults) => {
const granted = permissionResults.every((permissionResult) => permissionResult.granted)
const reason = granted
? ''
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {type Observable} from 'rxjs'
import {isEqual} from 'lodash'
import {useEffect, useReducer, useRef} from 'react'
import {distinctUntilChanged, type Observable, type Subscription} from 'rxjs'

import {createHookFromObservableFactory, type PartialExcept} from '../../../util'
import {type LoadingTuple, type PartialExcept} from '../../../util'
import {useGrantsStore} from '../datastores'
import {type DocumentValuePermission, type GrantsStore, type PermissionCheckResult} from './types'

Expand All @@ -11,23 +13,6 @@ export interface DocumentValuePermissionsOptions {
permission: DocumentValuePermission
}

/**
* The observable version of `useDocumentValuePermissions`
*
* @see useDocumentValuePermissions
*
* @internal
*/
export function getDocumentValuePermissions({
grantsStore,
document,
permission,
}: DocumentValuePermissionsOptions): Observable<PermissionCheckResult> {
const {checkDocumentPermission} = grantsStore

return checkDocumentPermission(permission, document)
}

/**
* Gets permissions based on the value of the document passed into the hook
* (stateless).
Expand All @@ -49,23 +34,83 @@ export function getDocumentValuePermissions({
*
* @internal
*/
export const useDocumentValuePermissionsFromHookFactory = createHookFromObservableFactory(
getDocumentValuePermissions,
)
export function getDocumentValuePermissions({
grantsStore,
document,
permission,
}: DocumentValuePermissionsOptions): Observable<PermissionCheckResult> {
const {checkDocumentPermission} = grantsStore

return checkDocumentPermission(permission, document)
}

const INITIAL_STATE: LoadingTuple<PermissionCheckResult | undefined> = [undefined, true]

function stateReducer(
prev: LoadingTuple<PermissionCheckResult | undefined>,
action:
| {type: 'loading'}
| {type: 'value'; value: PermissionCheckResult}
| {type: 'error'; error: unknown},
): LoadingTuple<PermissionCheckResult | undefined> {
const [prevResult, prevIsLoading] = prev
switch (action.type) {
// Keep the old value around while loading a new one. Prevents "jittering" UIs, and permissions
// usually don't change _that_ rapidly (this hook runs on every single document change).
case 'loading':
return [prevResult, true]
case 'value':
// Signal "no update" to React if we've got the same value as before by returning old state
return !prevIsLoading && isEqual(action.value, prevResult) ? prev : [action.value, false]
case 'error':
throw action.error
default:
throw new Error(`Invalid action type: ${action}`)
}
}

/** @internal */
export function useDocumentValuePermissions({
document,
permission,
...rest
}: PartialExcept<DocumentValuePermissionsOptions, 'permission' | 'document'>): ReturnType<
typeof useDocumentValuePermissionsFromHookFactory
grantsStore: specifiedGrantsStore,
}: PartialExcept<DocumentValuePermissionsOptions, 'permission' | 'document'>): LoadingTuple<
PermissionCheckResult | undefined
> {
const grantsStore = useGrantsStore()
const defaultGrantsStore = useGrantsStore()
const grantsStore = specifiedGrantsStore || defaultGrantsStore

const [state, dispatch] = useReducer(stateReducer, INITIAL_STATE)
const subscriptionRef = useRef<Subscription | null>(null)

useEffect(() => {
dispatch({type: 'loading'})

// Unsubscribe from any previous subscription
if (subscriptionRef.current) {
subscriptionRef.current.unsubscribe()
}

const permissions$ = getDocumentValuePermissions({
grantsStore,
document,
permission,
})

subscriptionRef.current = permissions$
.pipe(distinctUntilChanged((prev, next) => isEqual(prev, next)))
.subscribe({
next: (value) => dispatch({type: 'value', value}),
error: (error) => dispatch({type: 'error', error}),
})

return () => {
if (subscriptionRef.current) {
subscriptionRef.current.unsubscribe()
subscriptionRef.current = null
}
}
}, [grantsStore, document, permission])

return useDocumentValuePermissionsFromHookFactory({
grantsStore: rest.grantsStore || grantsStore,
document,
permission,
})
return state
}
2 changes: 1 addition & 1 deletion packages/sanity/src/core/store/_legacy/grants/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface GrantsStore {
/**
* Returns an observable of `PermissionCheckResult`
*
* This API is returns an observable (vs a promise) so the consumer can reac
* This API is returns an observable (vs a promise) so the consumer can react
* to incoming changes to the user permissions (e.g. for changing _debug_
* roles).
*
Expand Down
35 changes: 20 additions & 15 deletions packages/sanity/src/core/studio/screens/RequestAccessScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import {Box, Card, Flex, Stack, Text, TextInput, useToast} from '@sanity/ui'
import {addWeeks, isAfter, isBefore} from 'date-fns'
import {useCallback, useEffect, useState} from 'react'
import {finalize} from 'rxjs'
import {
type CurrentUser,
getProviderTitle,
Expand Down Expand Up @@ -73,16 +74,19 @@ export function RequestAccessScreen() {
}
}, [activeWorkspace])

// Check if user has a pending
// access request for this project
// Check if user has a pending access request for this project
useEffect(() => {
if (!client || !projectId) return
client
if (!client || !projectId) return () => {}
const request$ = client.observable
.request<AccessRequest[] | null>({
url: `/access/requests/me`,
url: '/access/requests/me',
tag: 'request-access',
})
.then((requests) => {
if (requests && requests?.length) {
.pipe(finalize(() => setLoading(false)))
.subscribe({
next: (requests) => {
if (!requests || !requests.length) return

const projectRequests = requests.filter((request) => request.resourceId === projectId)
const declinedRequest = projectRequests.find((request) => request.status === 'declined')
if (
Expand Down Expand Up @@ -111,15 +115,16 @@ export function RequestAccessScreen() {
if (oldPendingRequest) {
setExpiredHasPendingRequest(true)
}
}
})
.catch((err) => {
console.error(err)
setError(true)
})
.finally(() => {
setLoading(false)
},
error: (err) => {
console.error(err)
setError(true)
},
})

return () => {
request$.unsubscribe()
}
}, [client, projectId])

const handleSubmitRequest = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {addWeeks, isAfter, isBefore} from 'date-fns'
import {useMemo} from 'react'
import {useObservable} from 'react-rx'
import {from, of} from 'rxjs'
import {of} from 'rxjs'
import {catchError, map, startWith} from 'rxjs/operators'
import {useClient, useProjectId} from 'sanity'
import {type SanityClient, useClient, useProjectId} from 'sanity'

/** @internal */
export interface AccessRequest {
Expand All @@ -20,70 +20,75 @@ export interface AccessRequest {
note: string
}

const LOADING_STATE = {loading: true, error: false, status: undefined}
const EMPTY_STATE = {loading: false, error: false, status: 'none'}
const DECLINED_STATE = {loading: false, error: false, status: 'declined'}
const PENDING_STATE = {loading: false, error: false, status: 'pending'}
const EXPIRED_STATE = {loading: false, error: false, status: 'expired'}

/** @internal */
export const useRoleRequestsStatus = () => {
const client = useClient({apiVersion: '2024-07-01'})
const projectId = useProjectId()

const checkRoleRequests = useMemo(() => {
const checkRoleRequests$ = useMemo(() => {
if (!client || !projectId) {
return of({loading: false, error: false, status: 'none'})
return of(EMPTY_STATE)
}

return from(
client.request<AccessRequest[] | null>({
url: `/access/requests/me`,
}),
).pipe(
return checkRoleRequests(client, projectId)
}, [client, projectId])

const {loading, error, status} = useObservable(checkRoleRequests$, LOADING_STATE)
return {data: status, loading, error}
}

function checkRoleRequests(client: SanityClient, projectId: string) {
return client.observable
.request<AccessRequest[] | null>({
url: '/access/requests/me',
tag: 'use-role-requests-status',
})
.pipe(
map((requests) => {
if (requests && requests.length) {
// Filter requests for the specific project and where type is 'role'
const projectRequests = requests.filter(
(request) => request.resourceId === projectId && request.type === 'role',
)
if (!requests || requests.length === 0) {
return EMPTY_STATE
}

const declinedRequest = projectRequests.find((request) => request.status === 'declined')
if (
declinedRequest &&
isAfter(addWeeks(new Date(declinedRequest.createdAt), 2), new Date())
) {
return {loading: false, error: false, status: 'declined'}
}
// Filter requests for the specific project and where type is 'role'
const projectRequests = requests.filter(
(request) => request.resourceId === projectId && request.type === 'role',
)

const pendingRequest = projectRequests.find(
(request) =>
request.status === 'pending' &&
isAfter(addWeeks(new Date(request.createdAt), 2), new Date()),
)
if (pendingRequest) {
return {loading: false, error: false, status: 'pending'}
}
const declinedRequest = projectRequests.find((request) => request.status === 'declined')
if (
declinedRequest &&
isAfter(addWeeks(new Date(declinedRequest.createdAt), 2), new Date())
) {
return DECLINED_STATE
}

const oldPendingRequest = projectRequests.find(
(request) =>
request.status === 'pending' &&
isBefore(addWeeks(new Date(request.createdAt), 2), new Date()),
)
if (oldPendingRequest) {
return {loading: false, error: false, status: 'expired'}
}
const pendingRequest = projectRequests.find(
(request) =>
request.status === 'pending' &&
isAfter(addWeeks(new Date(request.createdAt), 2), new Date()),
)
if (pendingRequest) {
return PENDING_STATE
}
return {loading: false, error: false, status: 'none'}

const oldPendingRequest = projectRequests.find(
(request) =>
request.status === 'pending' &&
isBefore(addWeeks(new Date(request.createdAt), 2), new Date()),
)

return oldPendingRequest ? EXPIRED_STATE : EMPTY_STATE
}),
catchError((err) => {
console.error('Failed to fetch access requests', err)
return of({loading: false, error: true, status: undefined})
}),
startWith({loading: true, error: false, status: undefined}), // Start with loading state
startWith(LOADING_STATE), // Start with loading state
)
}, [client, projectId])

// Use useObservable to subscribe to the checkRoleRequests observable
const {loading, error, status} = useObservable(checkRoleRequests, {
loading: true,
error: false,
status: undefined,
})

return {data: status, loading, error}
}
Loading

0 comments on commit 45a74fc

Please sign in to comment.