diff --git a/.changeset/blue-beers-kiss.md b/.changeset/blue-beers-kiss.md new file mode 100644 index 0000000000..b05f99ebc9 --- /dev/null +++ b/.changeset/blue-beers-kiss.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-react': minor +--- + +**BREAKING**: More restrictive typing for `usePermission` hook and `PermissionedRoute` component. It's no longer possible to pass a `resourceRef` unless the permission is of type `ResourcPermission`. diff --git a/plugins/permission-react/api-report.md b/plugins/permission-react/api-report.md index 1bcb01fa32..81d9e09743 100644 --- a/plugins/permission-react/api-report.md +++ b/plugins/permission-react/api-report.md @@ -12,6 +12,7 @@ import { DiscoveryApi } from '@backstage/core-plugin-api'; import { IdentityApi } from '@backstage/core-plugin-api'; import { Permission } from '@backstage/plugin-permission-common'; import { ReactElement } from 'react'; +import { ResourcePermission } from '@backstage/plugin-permission-common'; import { Route } from 'react-router'; // @public (undocumented) @@ -44,17 +45,25 @@ export const permissionApiRef: ApiRef; // @public export const PermissionedRoute: ( props: ComponentProps & { - permission: Permission; - resourceRef?: string; errorComponent?: ReactElement | null; - }, + } & ( + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string | undefined; + } + ), ) => JSX.Element; // @public -export const usePermission: ( - permission: Permission, - resourceRef?: string | undefined, -) => AsyncPermissionResult; +export function usePermission( + ...[permission, resourceRef]: + | [ResourcePermission, string | undefined] + | [Exclude] +): AsyncPermissionResult; // (No @packageDocumentation comment for this package) ``` diff --git a/plugins/permission-react/src/components/PermissionedRoute.tsx b/plugins/permission-react/src/components/PermissionedRoute.tsx index 76017308aa..92b1830eca 100644 --- a/plugins/permission-react/src/components/PermissionedRoute.tsx +++ b/plugins/permission-react/src/components/PermissionedRoute.tsx @@ -18,7 +18,11 @@ import React, { ComponentProps, ReactElement } from 'react'; import { Route } from 'react-router'; import { useApp } from '@backstage/core-plugin-api'; import { usePermission } from '../hooks'; -import { Permission } from '@backstage/plugin-permission-common'; +import { + isResourcePermission, + Permission, + ResourcePermission, +} from '@backstage/plugin-permission-common'; /** * Returns a React Router Route which only renders the element when authorized. If unauthorized, the Route will render a @@ -28,13 +32,25 @@ import { Permission } from '@backstage/plugin-permission-common'; */ export const PermissionedRoute = ( props: ComponentProps & { - permission: Permission; - resourceRef?: string; errorComponent?: ReactElement | null; - }, + } & ( + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string | undefined; + } + ), ) => { const { permission, resourceRef, errorComponent, ...otherProps } = props; - const permissionResult = usePermission(permission, resourceRef); + + const permissionResult = usePermission( + ...(isResourcePermission(permission) + ? [permission, resourceRef] + : [permission]), + ); const app = useApp(); const { NotFoundErrorPage } = app.getComponents(); diff --git a/plugins/permission-react/src/hooks/usePermission.ts b/plugins/permission-react/src/hooks/usePermission.ts index cc27f82c59..f2895ffdcb 100644 --- a/plugins/permission-react/src/hooks/usePermission.ts +++ b/plugins/permission-react/src/hooks/usePermission.ts @@ -17,8 +17,11 @@ import { useApi } from '@backstage/core-plugin-api'; import { permissionApiRef } from '../apis'; import { + AuthorizeQuery, AuthorizeResult, + isResourcePermission, Permission, + ResourcePermission, } from '@backstage/plugin-permission-common'; import useSWR from 'swr'; @@ -30,25 +33,40 @@ export type AsyncPermissionResult = { }; /** - * React hook utility for authorization. Given a - * {@link @backstage/plugin-permission-common#Permission} and an optional - * resourceRef, it will return whether or not access is allowed (for the given - * resource, if resourceRef is provided). See + * React hook utility for authorization. Given either a non-resource + * {@link @backstage/plugin-permission-common#Permission} or a + * {@link @backstage/plugin-permission-common#ResourcePermission} and an + * optional resourceRef, it will return whether or not access is allowed (for + * the given resource, if resourceRef is provided). See * {@link @backstage/plugin-permission-common/PermissionClient#authorize} for * more details. * + * The resourceRef parameter is optional to allow calling this hook with an + * entity that might be loading asynchronously, but when resourceRef is not + * supplied, the value of `allowed` will always be false. + * * Note: This hook uses stale-while-revalidate to help avoid flicker in UI * elements that would be conditionally rendered based on the `allowed` result * of this hook. * @public */ -export const usePermission = ( - permission: Permission, - resourceRef?: string, -): AsyncPermissionResult => { +export function usePermission( + ...[permission, resourceRef]: + | [ResourcePermission, string | undefined] + | [Exclude] +): AsyncPermissionResult { const permissionApi = useApi(permissionApiRef); const { data, error } = useSWR({ permission, resourceRef }, async args => { - const { result } = await permissionApi.authorize(args); + // We could make the resourceRef parameter required to avoid this check, but + // it would make using this hook difficult in situations where the entity + // must be asynchronously loaded, so instead we short-circuit to a deny when + // no resourceRef is supplied, on the assumption that the resourceRef is + // still loading outside the hook. + if (isResourcePermission(args.permission) && !args.resourceRef) { + return AuthorizeResult.DENY; + } + + const { result } = await permissionApi.authorize(args as AuthorizeQuery); return result; }); @@ -59,4 +77,4 @@ export const usePermission = ( return { loading: true, allowed: false }; } return { loading: false, allowed: data === AuthorizeResult.ALLOW }; -}; +}