diff --git a/.changeset/cuddly-turtles-sleep.md b/.changeset/cuddly-turtles-sleep.md new file mode 100644 index 0000000000..a63cd44232 --- /dev/null +++ b/.changeset/cuddly-turtles-sleep.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Use new `PermissionEvaluator#authorizeConditional` method when retrieving permission conditions. diff --git a/.changeset/eight-cobras-think.md b/.changeset/eight-cobras-think.md new file mode 100644 index 0000000000..af1c7cd113 --- /dev/null +++ b/.changeset/eight-cobras-think.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search-backend': patch +--- + +Use `PermissionEvaluator` instead of `PermissionAuthorizer`, which is now deprecated. diff --git a/.changeset/few-seas-fail.md b/.changeset/few-seas-fail.md new file mode 100644 index 0000000000..c5d9db4d47 --- /dev/null +++ b/.changeset/few-seas-fail.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-permission-common': patch +--- + +Added `PermissionEvaluator`, which will replace the existing `PermissionAuthorizer` interface. This new interface provides stronger type safety and validation by splitting `PermissionAuthorizer.authorize()` into two methods: + +- `authorize()`: Used when the caller requires a definitive decision. +- `authorizeConditional()`: Used when the caller can optimize the evaluation of any conditional decisions. For example, a plugin backend may want to use conditions in a database query instead of evaluating each resource in memory. diff --git a/.changeset/four-dolphins-report.md b/.changeset/four-dolphins-report.md new file mode 100644 index 0000000000..fa3f06c532 --- /dev/null +++ b/.changeset/four-dolphins-report.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': minor +--- + +**BREAKING:** `ServerPermissionClient` now implements `PermissionEvaluator`, which moves out the capabilities for evaluating conditional decisions from `authorize()` to `authorizeConditional()` method. diff --git a/.changeset/fresh-boxes-pull.md b/.changeset/fresh-boxes-pull.md new file mode 100644 index 0000000000..e73d7e25c2 --- /dev/null +++ b/.changeset/fresh-boxes-pull.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-jenkins-backend': patch +--- + +Use `PermissionEvaluator` instead of `PermissionAuthorizer`, which is now deprecated. diff --git a/.changeset/rare-parents-pretend.md b/.changeset/rare-parents-pretend.md new file mode 100644 index 0000000000..f211a85efe --- /dev/null +++ b/.changeset/rare-parents-pretend.md @@ -0,0 +1,21 @@ +--- +'@backstage/create-app': patch +--- + +Accept `PermissionEvaluator` instead of the deprecated `PermissionAuthorizer`. + +Apply the following to `packages/backend/src/types.ts`: + +```diff +- import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; ++ import { PermissionEvaluator } from '@backstage/plugin-permission-common'; + + export type PluginEnvironment = { + ... + discovery: PluginEndpointDiscovery; + tokenManager: TokenManager; + scheduler: PluginTaskScheduler; +- permissions: PermissionAuthorizer; ++ permissions: PermissionEvaluator; + }; +``` diff --git a/.changeset/soft-rice-remember.md b/.changeset/soft-rice-remember.md new file mode 100644 index 0000000000..bf207a0dd5 --- /dev/null +++ b/.changeset/soft-rice-remember.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-react': patch +--- + +**BREAKING:** Make `IdentityPermissionApi#authorize` typing more strict, using `AuthorizePermissionRequest` and `AuthorizePermissionResponse`. diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index 0b2543c4f5..3e47b1a523 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -23,8 +23,11 @@ import { TokenManager, UrlReader, } from '@backstage/backend-common'; -import { ServerPermissionClient } from '@backstage/plugin-permission-node'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; +import { + PermissionAuthorizer, + PermissionEvaluator, +} from '@backstage/plugin-permission-common'; export type PluginEnvironment = { logger: Logger; @@ -34,6 +37,6 @@ export type PluginEnvironment = { reader: UrlReader; discovery: PluginEndpointDiscovery; tokenManager: TokenManager; - permissions: ServerPermissionClient; + permissions: PermissionEvaluator | PermissionAuthorizer; scheduler: PluginTaskScheduler; }; diff --git a/packages/create-app/templates/default-app/packages/backend/src/types.ts b/packages/create-app/templates/default-app/packages/backend/src/types.ts index 0862b0e874..8e0a86404b 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/types.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/types.ts @@ -8,7 +8,7 @@ import { UrlReader, } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; -import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; export type PluginEnvironment = { logger: Logger; @@ -19,5 +19,5 @@ export type PluginEnvironment = { discovery: PluginEndpointDiscovery; tokenManager: TokenManager; scheduler: PluginTaskScheduler; - permissions: PermissionAuthorizer; + permissions: PermissionEvaluator; }; diff --git a/plugins/catalog-backend/api-report.md b/plugins/catalog-backend/api-report.md index d44e744bb6..a770d6289e 100644 --- a/plugins/catalog-backend/api-report.md +++ b/plugins/catalog-backend/api-report.md @@ -22,6 +22,7 @@ import { Permission } from '@backstage/plugin-permission-common'; import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { PermissionRule } from '@backstage/plugin-permission-node'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; @@ -181,7 +182,7 @@ export type CatalogEnvironment = { database: PluginDatabaseManager; config: Config; reader: UrlReader; - permissions: PermissionAuthorizer; + permissions: PermissionEvaluator | PermissionAuthorizer; }; // @alpha diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts index b29e1121b3..51edd903e6 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts @@ -30,6 +30,7 @@ describe('AuthorizedEntitiesCatalog', () => { }; const fakePermissionApi = { authorize: jest.fn(), + authorizeConditional: jest.fn(), }; const createCatalog = (...rules: CatalogPermissionRule[]) => @@ -45,7 +46,7 @@ describe('AuthorizedEntitiesCatalog', () => { describe('entities', () => { it('returns empty response on DENY', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.DENY }, ]); const catalog = createCatalog(); @@ -61,7 +62,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); it('calls underlying catalog method with correct filter on CONDITIONAL', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, @@ -78,7 +79,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); it('calls underlying catalog method on ALLOW', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.ALLOW }, ]); const catalog = createCatalog(); @@ -98,7 +99,7 @@ describe('AuthorizedEntitiesCatalog', () => { { kind: 'component', namespace: 'default', name: 'my-component' }, ], }); - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.DENY }, ]); const catalog = new AuthorizedEntitiesCatalog( @@ -113,7 +114,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); it('throws error on CONDITIONAL authorization that evaluates to 0 entities', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, @@ -132,7 +133,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); it('calls underlying catalog method on CONDITIONAL authorization that evaluates to nonzero entities', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, @@ -158,7 +159,7 @@ describe('AuthorizedEntitiesCatalog', () => { { kind: 'component', namespace: 'default', name: 'my-component' }, ], }); - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.ALLOW }, ]); const catalog = new AuthorizedEntitiesCatalog( @@ -252,7 +253,7 @@ describe('AuthorizedEntitiesCatalog', () => { describe('facets', () => { it('returns empty response on DENY', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.DENY }, ]); const catalog = createCatalog(); @@ -268,7 +269,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); it('calls underlying catalog method with correct filter on CONDITIONAL', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, @@ -286,7 +287,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); it('calls underlying catalog method on ALLOW', async () => { - fakePermissionApi.authorize.mockResolvedValue([ + fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.ALLOW }, ]); const catalog = createCatalog(); diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts index 2f94e23303..0e861a39e1 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts @@ -22,7 +22,7 @@ import { import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { AuthorizeResult, - PermissionAuthorizer, + PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { ConditionTransformer } from '@backstage/plugin-permission-node'; import { @@ -39,13 +39,13 @@ import { basicEntityFilter } from './request/basicEntityFilter'; export class AuthorizedEntitiesCatalog implements EntitiesCatalog { constructor( private readonly entitiesCatalog: EntitiesCatalog, - private readonly permissionApi: PermissionAuthorizer, + private readonly permissionApi: PermissionEvaluator, private readonly transformConditions: ConditionTransformer, ) {} async entities(request?: EntitiesRequest): Promise { const authorizeDecision = ( - await this.permissionApi.authorize( + await this.permissionApi.authorizeConditional( [{ permission: catalogEntityReadPermission }], { token: request?.authorizationToken }, ) @@ -78,7 +78,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { options?: { authorizationToken?: string }, ): Promise { const authorizeResponse = ( - await this.permissionApi.authorize( + await this.permissionApi.authorizeConditional( [{ permission: catalogEntityDeletePermission }], { token: options?.authorizationToken }, ) @@ -155,7 +155,7 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { async facets(request: EntityFacetsRequest): Promise { const authorizeDecision = ( - await this.permissionApi.authorize( + await this.permissionApi.authorizeConditional( [{ permission: catalogEntityReadPermission }], { token: request?.authorizationToken }, ) diff --git a/plugins/catalog-backend/src/service/AuthorizedLocationService.test.ts b/plugins/catalog-backend/src/service/AuthorizedLocationService.test.ts index b27ca92924..dc8719bb0e 100644 --- a/plugins/catalog-backend/src/service/AuthorizedLocationService.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedLocationService.test.ts @@ -27,6 +27,7 @@ describe('AuthorizedLocationService', () => { }; const fakePermissionApi = { authorize: jest.fn(), + authorizeConditional: jest.fn(), }; const mockAllow = () => { diff --git a/plugins/catalog-backend/src/service/AuthorizedLocationService.ts b/plugins/catalog-backend/src/service/AuthorizedLocationService.ts index 077f56c737..a73597649e 100644 --- a/plugins/catalog-backend/src/service/AuthorizedLocationService.ts +++ b/plugins/catalog-backend/src/service/AuthorizedLocationService.ts @@ -24,14 +24,14 @@ import { } from '@backstage/plugin-catalog-common'; import { AuthorizeResult, - PermissionAuthorizer, + PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { LocationInput, LocationService } from './types'; export class AuthorizedLocationService implements LocationService { constructor( private readonly locationService: LocationService, - private readonly permissionApi: PermissionAuthorizer, + private readonly permissionApi: PermissionEvaluator, ) {} async createLocation( diff --git a/plugins/catalog-backend/src/service/AuthorizedRefreshService.test.ts b/plugins/catalog-backend/src/service/AuthorizedRefreshService.test.ts index 82bedec573..5d0a192470 100644 --- a/plugins/catalog-backend/src/service/AuthorizedRefreshService.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedRefreshService.test.ts @@ -25,6 +25,7 @@ describe('AuthorizedRefreshService', () => { }; const permissionApi = { authorize: jest.fn(), + query: jest.fn(), }; afterEach(() => { diff --git a/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts b/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts index 17dfb58c54..8634fbf86d 100644 --- a/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts +++ b/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts @@ -18,14 +18,14 @@ import { NotAllowedError } from '@backstage/errors'; import { catalogEntityRefreshPermission } from '@backstage/plugin-catalog-common'; import { AuthorizeResult, - PermissionAuthorizer, + PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { RefreshOptions, RefreshService } from './types'; export class AuthorizedRefreshService implements RefreshService { constructor( private readonly service: RefreshService, - private readonly permissionApi: PermissionAuthorizer, + private readonly permissionApi: PermissionEvaluator, ) {} async refresh(options: RefreshOptions) { diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 075fcd6a5d..f5f00cd0fc 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -79,7 +79,11 @@ import { CatalogPermissionRule, permissionRules as catalogPermissionRules, } from '../permissions/rules'; -import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { + PermissionAuthorizer, + PermissionEvaluator, + toPermissionEvaluator, +} from '@backstage/plugin-permission-common'; import { createConditionTransformer, createPermissionIntegrationRouter, @@ -95,7 +99,7 @@ export type CatalogEnvironment = { database: PluginDatabaseManager; config: Config; reader: UrlReader; - permissions: PermissionAuthorizer; + permissions: PermissionEvaluator | PermissionAuthorizer; }; /** @@ -376,9 +380,20 @@ export class CatalogBuilder { policy, }); const unauthorizedEntitiesCatalog = new DefaultEntitiesCatalog(dbClient); + + let permissionEvaluator: PermissionEvaluator; + if ('query' in permissions) { + permissionEvaluator = permissions as PermissionEvaluator; + } else { + logger.warn( + 'PermissionAuthorizer is deprecated. Please use an instance of PermissionEvaluator instead of PermissionAuthorizer in PluginEnvironment#permissions', + ); + permissionEvaluator = toPermissionEvaluator(permissions); + } + const entitiesCatalog = new AuthorizedEntitiesCatalog( unauthorizedEntitiesCatalog, - permissions, + permissionEvaluator, createConditionTransformer(this.permissionRules), ); const permissionIntegrationRouter = createPermissionIntegrationRouter({ @@ -428,11 +443,11 @@ export class CatalogBuilder { this.locationAnalyzer ?? new RepoLocationAnalyzer(logger, integrations); const locationService = new AuthorizedLocationService( new DefaultLocationService(locationStore, orchestrator), - permissions, + permissionEvaluator, ); const refreshService = new AuthorizedRefreshService( new DefaultRefreshService({ database: processingDatabase }), - permissions, + permissionEvaluator, ); const router = await createRouter({ entitiesCatalog, diff --git a/plugins/jenkins-backend/api-report.md b/plugins/jenkins-backend/api-report.md index 37e8f80a80..029f2c7af3 100644 --- a/plugins/jenkins-backend/api-report.md +++ b/plugins/jenkins-backend/api-report.md @@ -9,6 +9,7 @@ import { Config } from '@backstage/config'; import express from 'express'; import { Logger } from 'winston'; import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; // Warning: (ae-missing-release-tag) "createRouter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -98,6 +99,6 @@ export interface RouterOptions { // (undocumented) logger: Logger; // (undocumented) - permissions?: PermissionAuthorizer; + permissions?: PermissionEvaluator | PermissionAuthorizer; } ``` diff --git a/plugins/jenkins-backend/src/service/jenkinsApi.test.ts b/plugins/jenkins-backend/src/service/jenkinsApi.test.ts index a5453b5ebe..29da414470 100644 --- a/plugins/jenkins-backend/src/service/jenkinsApi.test.ts +++ b/plugins/jenkins-backend/src/service/jenkinsApi.test.ts @@ -49,6 +49,7 @@ const fakePermissionApi = { result: AuthorizeResult.ALLOW, }, ]), + authorizeConditional: jest.fn(), }; describe('JenkinsApi', () => { diff --git a/plugins/jenkins-backend/src/service/jenkinsApi.ts b/plugins/jenkins-backend/src/service/jenkinsApi.ts index 38ab20b17b..8069f57692 100644 --- a/plugins/jenkins-backend/src/service/jenkinsApi.ts +++ b/plugins/jenkins-backend/src/service/jenkinsApi.ts @@ -25,7 +25,7 @@ import type { } from '../types'; import { AuthorizeResult, - PermissionAuthorizer, + PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { jenkinsExecutePermission } from '@backstage/plugin-jenkins-common'; import { NotAllowedError } from '@backstage/errors'; @@ -64,7 +64,7 @@ export class JenkinsApiImpl { ${JenkinsApiImpl.jobTreeSpec} ]{0,50}`; - constructor(private readonly permissionApi?: PermissionAuthorizer) {} + constructor(private readonly permissionApi?: PermissionEvaluator) {} /** * Get a list of projects for the given JenkinsInfo. diff --git a/plugins/jenkins-backend/src/service/router.ts b/plugins/jenkins-backend/src/service/router.ts index 03462b923f..f6bd21c633 100644 --- a/plugins/jenkins-backend/src/service/router.ts +++ b/plugins/jenkins-backend/src/service/router.ts @@ -20,22 +20,38 @@ import Router from 'express-promise-router'; import { Logger } from 'winston'; import { JenkinsInfoProvider } from './jenkinsInfoProvider'; import { JenkinsApiImpl } from './jenkinsApi'; -import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { + PermissionAuthorizer, + PermissionEvaluator, + toPermissionEvaluator, +} from '@backstage/plugin-permission-common'; import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; import { stringifyEntityRef } from '@backstage/catalog-model'; export interface RouterOptions { logger: Logger; jenkinsInfoProvider: JenkinsInfoProvider; - permissions?: PermissionAuthorizer; + permissions?: PermissionEvaluator | PermissionAuthorizer; } export async function createRouter( options: RouterOptions, ): Promise { - const { jenkinsInfoProvider } = options; + const { jenkinsInfoProvider, permissions, logger } = options; - const jenkinsApi = new JenkinsApiImpl(options.permissions); + let permissionEvaluator: PermissionEvaluator | undefined; + if (permissions && 'query' in permissions) { + permissionEvaluator = permissions as PermissionEvaluator; + } else { + logger.warn( + 'PermissionAuthorizer is deprecated. Please use an instance of PermissionEvaluator instead of PermissionAuthorizer in PluginEnvironment#permissions', + ); + permissionEvaluator = permissions + ? toPermissionEvaluator(permissions) + : undefined; + } + + const jenkinsApi = new JenkinsApiImpl(permissionEvaluator); const router = Router(); router.use(express.json()); diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md index ab0c68470d..c0c351d6cd 100644 --- a/plugins/permission-common/api-report.md +++ b/plugins/permission-common/api-report.md @@ -15,6 +15,20 @@ export type AnyOfCriteria = { anyOf: NonEmptyArray>; }; +// @public +export type AuthorizePermissionRequest = + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string; + }; + +// @public +export type AuthorizePermissionResponse = DefinitivePolicyDecision; + // @public export type AuthorizeRequestOptions = { token?: string; @@ -78,6 +92,11 @@ export type EvaluatePermissionResponse = PolicyDecision; export type EvaluatePermissionResponseBatch = PermissionMessageBatch; +// @public +export type EvaluatorRequestOptions = { + token?: string; +}; + // @public export type IdentifiedPermissionMessage = T & { id: string; @@ -120,7 +139,7 @@ export type PermissionAttributes = { action?: 'create' | 'read' | 'update' | 'delete'; }; -// @public +// @public @deprecated export interface PermissionAuthorizer { // (undocumented) authorize( @@ -138,12 +157,16 @@ export type PermissionBase = { } & TFields; // @public -export class PermissionClient implements PermissionAuthorizer { +export class PermissionClient implements PermissionEvaluator { constructor(options: { discovery: DiscoveryApi; config: Config }); authorize( - queries: EvaluatePermissionRequest[], - options?: AuthorizeRequestOptions, - ): Promise; + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; + authorizeConditional( + queries: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; } // @public @@ -163,6 +186,18 @@ export type PermissionCriteria = | NotCriteria | TQuery; +// @public +export interface PermissionEvaluator { + authorize( + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; + authorizeConditional( + requests: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; +} + // @public export type PermissionMessageBatch = { items: IdentifiedPermissionMessage[]; @@ -173,6 +208,15 @@ export type PolicyDecision = | DefinitivePolicyDecision | ConditionalPolicyDecision; +// @public +export type QueryPermissionRequest = { + permission: ResourcePermission; + resourceRef?: never; +}; + +// @public +export type QueryPermissionResponse = PolicyDecision; + // @public export type ResourcePermission = PermissionBase< @@ -181,4 +225,9 @@ export type ResourcePermission = resourceType: TResourceType; } >; + +// @public +export function toPermissionEvaluator( + permissionAuthorizer: PermissionAuthorizer, +): PermissionEvaluator; ``` diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 65a7c7fbbe..503ce2768d 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -22,6 +22,7 @@ import { EvaluatePermissionRequest, AuthorizeResult, IdentifiedPermissionMessage, + ConditionalPolicyDecision, } from './types/api'; import { DiscoveryApi } from './types/discovery'; import { createPermission } from './permissions'; @@ -52,7 +53,7 @@ describe('PermissionClient', () => { afterEach(() => server.resetHandlers()); describe('authorize', () => { - const mockAuthorizeQuery = { + const mockAuthorizeConditional = { permission: mockPermission, resourceRef: 'foo:bar', }; @@ -77,12 +78,12 @@ describe('PermissionClient', () => { }); it('should fetch entities from correct endpoint', async () => { - await client.authorize([mockAuthorizeQuery]); + await client.authorize([mockAuthorizeConditional]); expect(mockAuthorizeHandler).toHaveBeenCalled(); }); it('should include a request body', async () => { - await client.authorize([mockAuthorizeQuery]); + await client.authorize([mockAuthorizeConditional]); const request = mockAuthorizeHandler.mock.calls[0][0]; @@ -97,21 +98,21 @@ describe('PermissionClient', () => { }); it('should return the response from the fetch request', async () => { - const response = await client.authorize([mockAuthorizeQuery]); + const response = await client.authorize([mockAuthorizeConditional]); expect(response[0]).toEqual( expect.objectContaining({ result: AuthorizeResult.ALLOW }), ); }); it('should not include authorization headers if no token is supplied', async () => { - await client.authorize([mockAuthorizeQuery]); + await client.authorize([mockAuthorizeConditional]); const request = mockAuthorizeHandler.mock.calls[0][0]; expect(request.headers.has('authorization')).toEqual(false); }); it('should include correctly-constructed authorization header if token is supplied', async () => { - await client.authorize([mockAuthorizeQuery], { token }); + await client.authorize([mockAuthorizeConditional], { token }); const request = mockAuthorizeHandler.mock.calls[0][0]; expect(request.headers.get('authorization')).toEqual('Bearer fake-token'); @@ -124,7 +125,7 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeQuery], { token }), + client.authorize([mockAuthorizeConditional], { token }), ).rejects.toThrowError(/request failed with 401/i); }); @@ -139,8 +140,8 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeQuery], { token }), - ).rejects.toThrowError(/Unexpected authorization response/i); + client.authorize([mockAuthorizeConditional], { token }), + ).rejects.toThrowError(/items in response do not match request/i); }); it('should reject invalid responses', async () => { @@ -157,7 +158,7 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeQuery], { token }), + client.authorize([mockAuthorizeConditional], { token }), ).rejects.toThrowError(/invalid input/i); }); @@ -178,7 +179,7 @@ describe('PermissionClient', () => { discovery, config: new ConfigReader({ permission: { enabled: false } }), }); - const response = await disabled.authorize([mockAuthorizeQuery]); + const response = await disabled.authorize([mockAuthorizeConditional]); expect(response[0]).toEqual( expect.objectContaining({ result: AuthorizeResult.ALLOW }), ); @@ -202,11 +203,206 @@ describe('PermissionClient', () => { discovery, config: new ConfigReader({}), }); - const response = await disabled.authorize([mockAuthorizeQuery]); + const response = await disabled.authorize([mockAuthorizeConditional]); expect(response[0]).toEqual( expect.objectContaining({ result: AuthorizeResult.ALLOW }), ); expect(mockAuthorizeHandler).not.toBeCalled(); }); }); + + describe('authorizeConditional', () => { + const mockResourceAuthorizeConditional = { + permission: mockPermission, + }; + + const mockPolicyDecisionHandler = jest.fn( + (req, res, { json }: RestContext) => { + const responses = req.body.items.map( + (a: IdentifiedPermissionMessage) => ({ + id: a.id, + pluginId: 'test-plugin', + resourceType: 'test-resource', + result: AuthorizeResult.CONDITIONAL, + conditions: { + resourceType: 'test-resource', + rule: 'FOO', + params: ['bar'], + }, + }), + ); + + return res(json({ items: responses })); + }, + ); + + beforeEach(() => { + server.use( + rest.post(`${mockBaseUrl}/authorize`, mockPolicyDecisionHandler), + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should fetch entities from correct endpoint', async () => { + await client.authorizeConditional([mockResourceAuthorizeConditional]); + expect(mockPolicyDecisionHandler).toHaveBeenCalled(); + }); + + it('should include a request body', async () => { + await client.authorizeConditional([mockResourceAuthorizeConditional]); + + const request = mockPolicyDecisionHandler.mock.calls[0][0]; + + expect(request.body).toEqual({ + items: [ + expect.objectContaining({ + permission: mockPermission, + }), + ], + }); + }); + + it('should return the response from the fetch request', async () => { + const response = await client.authorizeConditional([ + mockResourceAuthorizeConditional, + ]); + expect(response[0]).toEqual( + expect.objectContaining({ + result: AuthorizeResult.CONDITIONAL, + conditions: { + rule: 'FOO', + resourceType: 'test-resource', + params: ['bar'], + }, + }), + ); + }); + + it('should not include authorization headers if no token is supplied', async () => { + await client.authorizeConditional([mockResourceAuthorizeConditional]); + + const request = mockPolicyDecisionHandler.mock.calls[0][0]; + expect(request.headers.has('authorization')).toEqual(false); + }); + + it('should include correctly-constructed authorization header if token is supplied', async () => { + await client.authorizeConditional([mockResourceAuthorizeConditional], { + token, + }); + + const request = mockPolicyDecisionHandler.mock.calls[0][0]; + expect(request.headers.get('authorization')).toEqual('Bearer fake-token'); + }); + + it('should forward response errors', async () => { + mockPolicyDecisionHandler.mockImplementationOnce( + (_req, res, { status }: RestContext) => { + return res(status(401)); + }, + ); + await expect( + client.authorizeConditional([mockResourceAuthorizeConditional], { + token, + }), + ).rejects.toThrowError(/request failed with 401/i); + }); + + it('should reject responses with missing ids', async () => { + mockPolicyDecisionHandler.mockImplementationOnce( + (_req, res, { json }: RestContext) => { + return res( + json({ + items: [{ id: 'wrong-id', result: AuthorizeResult.ALLOW }], + }), + ); + }, + ); + await expect( + client.authorizeConditional([mockResourceAuthorizeConditional], { + token, + }), + ).rejects.toThrowError(/items in response do not match request/i); + }); + + it('should reject invalid responses', async () => { + mockPolicyDecisionHandler.mockImplementationOnce( + (req, res, { json }: RestContext) => { + const responses = req.body.items.map( + (a: IdentifiedPermissionMessage) => ({ + id: a.id, + outcome: AuthorizeResult.ALLOW, + }), + ); + + return res(json({ items: responses })); + }, + ); + await expect( + client.authorizeConditional([mockResourceAuthorizeConditional], { + token, + }), + ).rejects.toThrowError(/invalid input/i); + }); + + it('should allow all when permission.enabled is false', async () => { + mockPolicyDecisionHandler.mockImplementationOnce( + (req, res, { json }: RestContext) => { + const responses = req.body.map( + (a: IdentifiedPermissionMessage) => ({ + id: a.id, + result: AuthorizeResult.DENY, + }), + ); + + return res(json({ items: responses })); + }, + ); + const disabled = new PermissionClient({ + discovery, + config: new ConfigReader({ permission: { enabled: false } }), + }); + const response = await disabled.authorizeConditional( + [mockResourceAuthorizeConditional], + { + token, + }, + ); + expect(response[0]).toEqual( + expect.objectContaining({ result: AuthorizeResult.ALLOW }), + ); + expect(mockPolicyDecisionHandler).not.toBeCalled(); + }); + + it('should allow all when permission.enabled is not configured', async () => { + mockPolicyDecisionHandler.mockImplementationOnce( + (req, res, { json }: RestContext) => { + const responses = req.body.map( + (a: IdentifiedPermissionMessage) => ({ + id: a.id, + outcome: AuthorizeResult.DENY, + }), + ); + + return res(json(responses)); + }, + ); + const disabled = new PermissionClient({ + discovery, + config: new ConfigReader({}), + }); + const response = await disabled.authorizeConditional( + [mockResourceAuthorizeConditional], + { + token, + }, + ); + expect(response[0]).toEqual( + expect.objectContaining({ result: AuthorizeResult.ALLOW }), + ); + expect(mockPolicyDecisionHandler).not.toBeCalled(); + }); + }); }); diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 72f6b04567..223209c69f 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -21,19 +21,18 @@ import * as uuid from 'uuid'; import { z } from 'zod'; import { AuthorizeResult, - EvaluatePermissionRequest, - EvaluatePermissionResponse, - IdentifiedPermissionMessage, + PermissionMessageBatch, PermissionCriteria, PermissionCondition, - EvaluatePermissionResponseBatch, - EvaluatePermissionRequestBatch, + PermissionEvaluator, + QueryPermissionRequest, + AuthorizePermissionRequest, + EvaluatorRequestOptions, + AuthorizePermissionResponse, + QueryPermissionResponse, } from './types/api'; import { DiscoveryApi } from './types/discovery'; -import { - PermissionAuthorizer, - AuthorizeRequestOptions, -} from './types/permission'; +import { AuthorizeRequestOptions } from './types/permission'; const permissionCriteriaSchema: z.ZodSchema< PermissionCriteria @@ -58,30 +57,56 @@ const permissionCriteriaSchema: z.ZodSchema< .or(z.object({ not: permissionCriteriaSchema }).strict()), ); -const responseSchema = z.object({ - items: z.array( - z - .object({ - id: z.string(), - result: z - .literal(AuthorizeResult.ALLOW) - .or(z.literal(AuthorizeResult.DENY)), - }) - .or( - z.object({ - id: z.string(), - result: z.literal(AuthorizeResult.CONDITIONAL), - conditions: permissionCriteriaSchema, - }), +const authorizePermissionResponseSchema: z.ZodSchema = + z.object({ + result: z + .literal(AuthorizeResult.ALLOW) + .or(z.literal(AuthorizeResult.DENY)), + }); + +const queryPermissionResponseSchema: z.ZodSchema = + z.union([ + z.object({ + result: z + .literal(AuthorizeResult.ALLOW) + .or(z.literal(AuthorizeResult.DENY)), + }), + z.object({ + result: z.literal(AuthorizeResult.CONDITIONAL), + pluginId: z.string(), + resourceType: z.string(), + conditions: permissionCriteriaSchema, + }), + ]); + +const responseSchema = ( + itemSchema: z.ZodSchema, + ids: Set, +): z.ZodSchema> => + z.object({ + items: z + .array( + z.intersection( + z.object({ + id: z.string(), + }), + itemSchema, + ), + ) + .refine( + items => + items.length === ids.size && items.every(({ id }) => ids.has(id)), + { + message: 'Items in response do not match request', + }, ), - ), -}); + }); /** * An isomorphic client for requesting authorization for Backstage permissions. * @public */ -export class PermissionClient implements PermissionAuthorizer { +export class PermissionClient implements PermissionEvaluator { private readonly enabled: boolean; private readonly discovery: DiscoveryApi; @@ -92,35 +117,39 @@ export class PermissionClient implements PermissionAuthorizer { } /** - * Request authorization from the permission-backend for the given set of permissions. - * - * Authorization requests check that a given Backstage user can perform a protected operation, - * potentially for a specific resource (such as a catalog entity). The Backstage identity token - * should be included in the `options` if available. - * - * Permissions can be imported from plugins exposing them, such as `catalogEntityReadPermission`. - * - * The response will be either ALLOW or DENY when either the permission has no resourceType, or a - * resourceRef is provided in the request. For permissions with a resourceType, CONDITIONAL may be - * returned if no resourceRef is provided in the request. Conditional responses are intended only - * for backends which have access to the data source for permissioned resources, so that filters - * can be applied when loading collections of resources. - * @public + * {@inheritdoc PermissionEvaluator.authorize} */ async authorize( - queries: EvaluatePermissionRequest[], - options?: AuthorizeRequestOptions, - ): Promise { - // TODO(permissions): it would be great to provide some kind of typing guarantee that - // conditional responses will only ever be returned for requests containing a resourceType - // but no resourceRef. That way clients who aren't prepared to handle filtering according - // to conditions can be guaranteed that they won't unexpectedly get a CONDITIONAL response. + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise { + return this.makeRequest( + requests, + authorizePermissionResponseSchema, + options, + ); + } + /** + * {@inheritdoc PermissionEvaluator.authorizeConditional} + */ + async authorizeConditional( + queries: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise { + return this.makeRequest(queries, queryPermissionResponseSchema, options); + } + + private async makeRequest( + queries: TQuery[], + itemSchema: z.ZodSchema, + options?: AuthorizeRequestOptions, + ) { if (!this.enabled) { - return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); + return queries.map(_ => ({ result: AuthorizeResult.ALLOW as const })); } - const request: EvaluatePermissionRequestBatch = { + const request: PermissionMessageBatch = { items: queries.map(query => ({ id: uuid.v4(), ...query, @@ -141,12 +170,16 @@ export class PermissionClient implements PermissionAuthorizer { } const responseBody = await response.json(); - this.assertValidResponse(request, responseBody); - const responsesById = responseBody.items.reduce((acc, r) => { + const parsedResponse = responseSchema( + itemSchema, + new Set(request.items.map(({ id }) => id)), + ).parse(responseBody); + + const responsesById = parsedResponse.items.reduce((acc, r) => { acc[r.id] = r; return acc; - }, {} as Record>); + }, {} as Record>); return request.items.map(query => responsesById[query.id]); } @@ -154,20 +187,4 @@ export class PermissionClient implements PermissionAuthorizer { private getAuthorizationHeader(token?: string): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } - - private assertValidResponse( - request: EvaluatePermissionRequestBatch, - json: any, - ): asserts json is EvaluatePermissionResponseBatch { - const authorizedResponses = responseSchema.parse(json); - const responseIds = authorizedResponses.items.map(r => r.id); - const hasAllRequestIds = request.items.every(r => - responseIds.includes(r.id), - ); - if (!hasAllRequestIds) { - throw new Error( - 'Unexpected authorization response from permission-backend', - ); - } - } } diff --git a/plugins/permission-common/src/permissions/util.ts b/plugins/permission-common/src/permissions/util.ts index 1a5ed434a7..5cc9d9c08a 100644 --- a/plugins/permission-common/src/permissions/util.ts +++ b/plugins/permission-common/src/permissions/util.ts @@ -14,7 +14,18 @@ * limitations under the License. */ -import { Permission, ResourcePermission } from '../types'; +import { + AuthorizePermissionRequest, + AuthorizePermissionResponse, + DefinitivePolicyDecision, + EvaluatorRequestOptions, + Permission, + PermissionAuthorizer, + PermissionEvaluator, + QueryPermissionRequest, + QueryPermissionResponse, + ResourcePermission, +} from '../types'; /** * Check if the two parameters are equivalent permissions. @@ -75,3 +86,31 @@ export function isUpdatePermission(permission: Permission) { export function isDeletePermission(permission: Permission) { return permission.attributes.action === 'delete'; } + +/** + * Convert {@link PermissionAuthorizer} to {@link PermissionEvaluator}. + * + * @public + */ +export function toPermissionEvaluator( + permissionAuthorizer: PermissionAuthorizer, +): PermissionEvaluator { + return { + authorize: async ( + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise => { + const response = await permissionAuthorizer.authorize(requests, options); + + return response as DefinitivePolicyDecision[]; + }, + authorizeConditional( + requests: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise { + const parsedRequests = + requests as unknown as AuthorizePermissionRequest[]; + return permissionAuthorizer.authorize(parsedRequests, options); + }, + }; +} diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index 6aaabf978d..f29a639d18 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { ResourcePermission } from '.'; import { Permission } from './permission'; /** @@ -182,3 +183,73 @@ export type EvaluatePermissionResponse = PolicyDecision; */ export type EvaluatePermissionResponseBatch = PermissionMessageBatch; + +/** + * Request object for {@link PermissionEvaluator.authorize}. If a {@link ResourcePermission} + * is provided, it must include a corresponding `resourceRef`. + * @public + */ +export type AuthorizePermissionRequest = + | { + permission: Exclude; + resourceRef?: never; + } + | { permission: ResourcePermission; resourceRef: string }; + +/** + * Response object for {@link PermissionEvaluator.authorize}. + * @public + */ +export type AuthorizePermissionResponse = DefinitivePolicyDecision; + +/** + * Request object for {@link PermissionEvaluator.authorizeConditional}. + * @public + */ +export type QueryPermissionRequest = { + permission: ResourcePermission; + resourceRef?: never; +}; + +/** + * Response object for {@link PermissionEvaluator.authorizeConditional}. + * @public + */ +export type QueryPermissionResponse = PolicyDecision; + +/** + * A client interacting with the permission backend can implement this evaluator interface. + * + * @public + */ +export interface PermissionEvaluator { + /** + * Evaluates {@link Permission | Permissions} and returns a definitive decision. + */ + authorize( + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; + + /** + * Evaluates {@link ResourcePermission | ResourcePermissions} and returns both definitive and + * conditional decisions, depending on the configured + * {@link @backstage/plugin-permission-node#PermissionPolicy}. This method is useful when the + * caller needs more control over the processing of conditional decisions. For example, a plugin + * backend may want to use {@link PermissionCriteria | conditions} in a database query instead of + * evaluating each resource in memory. + */ + authorizeConditional( + requests: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; +} + +/** + * Options for {@link PermissionEvaluator} requests. + * The Backstage identity token should be defined if available. + * @public + */ +export type EvaluatorRequestOptions = { + token?: string; +}; diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index 1a993a981c..f244a70b1a 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -22,6 +22,12 @@ export type { EvaluatePermissionResponseBatch, IdentifiedPermissionMessage, PermissionMessageBatch, + AuthorizePermissionRequest, + AuthorizePermissionResponse, + QueryPermissionRequest, + QueryPermissionResponse, + EvaluatorRequestOptions, + PermissionEvaluator, ConditionalPolicyDecision, DefinitivePolicyDecision, PolicyDecision, diff --git a/plugins/permission-common/src/types/permission.ts b/plugins/permission-common/src/types/permission.ts index 34e7884793..eb1c0eb50d 100644 --- a/plugins/permission-common/src/types/permission.ts +++ b/plugins/permission-common/src/types/permission.ts @@ -91,6 +91,7 @@ export type ResourcePermission = /** * A client interacting with the permission backend can implement this authorizer interface. * @public + * @deprecated Use {@link @backstage/plugin-permission-common#PermissionEvaluator} instead */ export interface PermissionAuthorizer { authorize( diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 57379dd68b..b2e4ff45b8 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -5,22 +5,23 @@ ```ts import { AllOfCriteria } from '@backstage/plugin-permission-common'; import { AnyOfCriteria } from '@backstage/plugin-permission-common'; -import { AuthorizeRequestOptions } from '@backstage/plugin-permission-common'; +import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common'; +import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; import { DefinitivePolicyDecision } from '@backstage/plugin-permission-common'; -import { EvaluatePermissionRequest } from '@backstage/plugin-permission-common'; -import { EvaluatePermissionResponse } from '@backstage/plugin-permission-common'; +import { EvaluatorRequestOptions } from '@backstage/plugin-permission-common'; import express from 'express'; import { IdentifiedPermissionMessage } from '@backstage/plugin-permission-common'; import { NotCriteria } from '@backstage/plugin-permission-common'; import { Permission } from '@backstage/plugin-permission-common'; -import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { PolicyDecision } from '@backstage/plugin-permission-common'; +import { QueryPermissionRequest } from '@backstage/plugin-permission-common'; import { ResourcePermission } from '@backstage/plugin-permission-common'; import { TokenManager } from '@backstage/backend-common'; @@ -178,12 +179,17 @@ export type PolicyQuery = { }; // @public -export class ServerPermissionClient implements PermissionAuthorizer { +export class ServerPermissionClient implements PermissionEvaluator { // (undocumented) authorize( - requests: EvaluatePermissionRequest[], - options?: AuthorizeRequestOptions, - ): Promise; + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; + // (undocumented) + authorizeConditional( + queries: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise; // (undocumented) static fromConfig( config: Config, diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index f5711ed4a0..e531bd3556 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -17,9 +17,10 @@ import { ServerPermissionClient } from './ServerPermissionClient'; import { IdentifiedPermissionMessage, - EvaluatePermissionRequest, AuthorizeResult, createPermission, + DefinitivePolicyDecision, + ConditionalPolicyDecision, } from '@backstage/plugin-permission-common'; import { ConfigReader } from '@backstage/config'; import { @@ -31,16 +32,7 @@ import { setupServer } from 'msw/node'; import { RestContext, rest } from 'msw'; const server = setupServer(); -const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { - const responses = req.body.items.map( - (r: IdentifiedPermissionMessage) => ({ - id: r.id, - result: AuthorizeResult.ALLOW, - }), - ); - return res(json({ items: responses })); -}); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; const discovery: PluginEndpointDiscovery = { async getBaseUrl() { @@ -50,10 +42,17 @@ const discovery: PluginEndpointDiscovery = { return mockBaseUrl; }, }; -const testPermission = createPermission({ +const testBasicPermission = createPermission({ name: 'test.permission', attributes: {}, }); + +const testResourcePermission = createPermission({ + name: 'test.permission-2', + attributes: {}, + resourceType: 'resource-type', +}); + const config = new ConfigReader({ permission: { enabled: true }, backend: { auth: { keys: [{ secret: 'a-secret-key' }] } }, @@ -63,49 +62,6 @@ const logger = getVoidLogger(); describe('ServerPermissionClient', () => { beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); afterAll(() => server.close()); - beforeEach(() => { - server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler)); - }); - afterEach(() => server.resetHandlers()); - - it('should bypass authorization if permissions are disabled', async () => { - const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { - discovery, - tokenManager: ServerTokenManager.noop(), - }); - - await client.authorize([{ permission: testPermission }]); - - expect(mockAuthorizeHandler).not.toHaveBeenCalled(); - }); - - it('should bypass authorization if permissions are enabled and request has valid server token', async () => { - const tokenManager = ServerTokenManager.fromConfig(config, { logger }); - const client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager, - }); - - await client.authorize([{ permission: testPermission }], { - token: (await tokenManager.getToken()).token, - }); - - expect(mockAuthorizeHandler).not.toHaveBeenCalled(); - }); - - it('should authorize normally if permissions are enabled and request does not have valid server token', async () => { - const tokenManager = ServerTokenManager.fromConfig(config, { logger }); - const client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager, - }); - - await client.authorize([{ permission: testPermission }], { - token: 'a-user-token', - }); - - expect(mockAuthorizeHandler).toHaveBeenCalled(); - }); it('should error if permissions are enabled but a no-op token manager is configured', async () => { expect(() => @@ -117,4 +73,134 @@ describe('ServerPermissionClient', () => { 'Backend-to-backend authentication must be configured before enabling permissions. Read more here https://backstage.io/docs/tutorials/backend-to-backend-auth', ); }); + + describe('authorize', () => { + let mockAuthorizeHandler: jest.Mock; + + beforeEach(() => { + mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { + const responses = req.body.items.map( + (r: IdentifiedPermissionMessage) => ({ + id: r.id, + result: AuthorizeResult.ALLOW, + }), + ); + + return res(json({ items: responses })); + }); + + server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler)); + }); + afterEach(() => server.resetHandlers()); + + it('should bypass the permission backend if permissions are disabled', async () => { + const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { + discovery, + tokenManager: ServerTokenManager.noop(), + }); + + await client.authorize([ + { + permission: testBasicPermission, + }, + ]); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => { + const tokenManager = ServerTokenManager.fromConfig(config, { logger }); + const client = ServerPermissionClient.fromConfig(config, { + discovery, + tokenManager, + }); + + await client.authorize([{ permission: testBasicPermission }], { + token: (await tokenManager.getToken()).token, + }); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => { + const tokenManager = ServerTokenManager.fromConfig(config, { logger }); + const client = ServerPermissionClient.fromConfig(config, { + discovery, + tokenManager, + }); + + await client.authorize([{ permission: testBasicPermission }], { + token: 'a-user-token', + }); + + expect(mockAuthorizeHandler).toHaveBeenCalled(); + }); + }); + + describe('authorizeConditional', () => { + let mockAuthorizeHandler: jest.Mock; + + beforeEach(() => { + mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { + const responses = req.body.items.map( + (r: IdentifiedPermissionMessage) => ({ + id: r.id, + result: AuthorizeResult.ALLOW, + }), + ); + + return res(json({ items: responses })); + }); + + server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler)); + }); + afterEach(() => server.resetHandlers()); + + it('should bypass the permission backend if permissions are disabled', async () => { + const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { + discovery, + tokenManager: ServerTokenManager.noop(), + }); + + await client.authorizeConditional([ + { permission: testResourcePermission }, + ]); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => { + const tokenManager = ServerTokenManager.fromConfig(config, { logger }); + const client = ServerPermissionClient.fromConfig(config, { + discovery, + tokenManager, + }); + + await client.authorizeConditional( + [{ permission: testResourcePermission }], + { + token: (await tokenManager.getToken()).token, + }, + ); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => { + const tokenManager = ServerTokenManager.fromConfig(config, { logger }); + const client = ServerPermissionClient.fromConfig(config, { + discovery, + tokenManager, + }); + + await client.authorizeConditional( + [{ permission: testResourcePermission }], + { + token: 'a-user-token', + }, + ); + + expect(mockAuthorizeHandler).toHaveBeenCalled(); + }); + }); }); diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index 56381dff17..664e5a8309 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -20,12 +20,14 @@ import { } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import { - EvaluatePermissionRequest, - AuthorizeRequestOptions, - EvaluatePermissionResponse, AuthorizeResult, PermissionClient, - PermissionAuthorizer, + PermissionEvaluator, + AuthorizePermissionRequest, + EvaluatorRequestOptions, + AuthorizePermissionResponse, + PolicyDecision, + QueryPermissionRequest, } from '@backstage/plugin-permission-common'; /** @@ -34,7 +36,7 @@ import { * backend-to-backend requests. * @public */ -export class ServerPermissionClient implements PermissionAuthorizer { +export class ServerPermissionClient implements PermissionEvaluator { private readonly permissionClient: PermissionClient; private readonly tokenManager: TokenManager; private readonly permissionEnabled: boolean; @@ -77,21 +79,22 @@ export class ServerPermissionClient implements PermissionAuthorizer { this.permissionEnabled = options.permissionEnabled; } + async authorizeConditional( + queries: QueryPermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise { + return (await this.isEnabled(options?.token)) + ? this.permissionClient.authorizeConditional(queries, options) + : queries.map(_ => ({ result: AuthorizeResult.ALLOW })); + } + async authorize( - requests: EvaluatePermissionRequest[], - options?: AuthorizeRequestOptions, - ): Promise { - // Check if permissions are enabled before validating the server token. That - // way when permissions are disabled, the noop token manager can be used - // without fouling up the logic inside the ServerPermissionClient, because - // the code path won't be reached. - if ( - !this.permissionEnabled || - (await this.isValidServerToken(options?.token)) - ) { - return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); - } - return this.permissionClient.authorize(requests, options); + requests: AuthorizePermissionRequest[], + options?: EvaluatorRequestOptions, + ): Promise { + return (await this.isEnabled(options?.token)) + ? this.permissionClient.authorize(requests, options) + : requests.map(_ => ({ result: AuthorizeResult.ALLOW })); } private async isValidServerToken( @@ -105,4 +108,12 @@ export class ServerPermissionClient implements PermissionAuthorizer { .then(() => true) .catch(() => false); } + + private async isEnabled(token?: string) { + // Check if permissions are enabled before validating the server token. That + // way when permissions are disabled, the noop token manager can be used + // without fouling up the logic inside the ServerPermissionClient, because + // the code path won't be reached. + return this.permissionEnabled && !(await this.isValidServerToken(token)); + } } diff --git a/plugins/permission-react/api-report.md b/plugins/permission-react/api-report.md index 17b29a463a..12f1300c70 100644 --- a/plugins/permission-react/api-report.md +++ b/plugins/permission-react/api-report.md @@ -4,6 +4,8 @@ ```ts import { ApiRef } from '@backstage/core-plugin-api'; +import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common'; +import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; import { ComponentProps } from 'react'; import { Config } from '@backstage/config'; import { DiscoveryApi } from '@backstage/core-plugin-api'; @@ -26,8 +28,8 @@ export type AsyncPermissionResult = { export class IdentityPermissionApi implements PermissionApi { // (undocumented) authorize( - request: EvaluatePermissionRequest, - ): Promise; + request: AuthorizePermissionRequest, + ): Promise; // (undocumented) static create(options: { config: Config; diff --git a/plugins/permission-react/src/apis/IdentityPermissionApi.ts b/plugins/permission-react/src/apis/IdentityPermissionApi.ts index 80d4eb8859..1fe295c9f2 100644 --- a/plugins/permission-react/src/apis/IdentityPermissionApi.ts +++ b/plugins/permission-react/src/apis/IdentityPermissionApi.ts @@ -17,8 +17,8 @@ import { DiscoveryApi, IdentityApi } from '@backstage/core-plugin-api'; import { PermissionApi } from './PermissionApi'; import { - EvaluatePermissionRequest, - EvaluatePermissionResponse, + AuthorizePermissionRequest, + AuthorizePermissionResponse, PermissionClient, } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; @@ -45,8 +45,8 @@ export class IdentityPermissionApi implements PermissionApi { } async authorize( - request: EvaluatePermissionRequest, - ): Promise { + request: AuthorizePermissionRequest, + ): Promise { const response = await this.permissionClient.authorize( [request], await this.identityApi.getCredentials(), diff --git a/plugins/search-backend/api-report.md b/plugins/search-backend/api-report.md index 3c7f9c2754..c5c8eb7a13 100644 --- a/plugins/search-backend/api-report.md +++ b/plugins/search-backend/api-report.md @@ -8,6 +8,7 @@ import { DocumentTypeInfo } from '@backstage/plugin-search-common'; import express from 'express'; import { Logger } from 'winston'; import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { SearchEngine } from '@backstage/plugin-search-backend-node'; // Warning: (ae-missing-release-tag) "createRouter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -21,7 +22,7 @@ export function createRouter(options: RouterOptions): Promise; export type RouterOptions = { engine: SearchEngine; types: Record; - permissions: PermissionAuthorizer; + permissions: PermissionEvaluator | PermissionAuthorizer; config: Config; logger: Logger; }; diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts index 47503312fb..256608fd82 100644 --- a/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts +++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts @@ -19,7 +19,8 @@ import { EvaluatePermissionResponse, AuthorizeResult, createPermission, - PermissionAuthorizer, + PolicyDecision, + PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { DocumentTypeInfo, @@ -69,12 +70,15 @@ describe('AuthorizedSearchEngine', () => { query: mockedQuery, }; - const mockedAuthorize: jest.MockedFunction< - PermissionAuthorizer['authorize'] + const mockedAuthorize: jest.MockedFunction = + jest.fn(); + const mockedPermissionQuery: jest.MockedFunction< + PermissionEvaluator['authorizeConditional'] > = jest.fn(); - const permissionAuthorizer: PermissionAuthorizer = { + const permissionEvaluator: PermissionEvaluator = { authorize: mockedAuthorize, + authorizeConditional: mockedPermissionQuery, }; const defaultTypes: Record = { @@ -111,13 +115,14 @@ describe('AuthorizedSearchEngine', () => { const authorizedSearchEngine = new AuthorizedSearchEngine( searchEngine, defaultTypes, - permissionAuthorizer, + permissionEvaluator, new ConfigReader({}), ); const options = { token: 'token' }; - const allowAll: PermissionAuthorizer['authorize'] = async queries => { + const allowAll: PermissionEvaluator['authorize'] & + PermissionEvaluator['authorizeConditional'] = async queries => { return queries.map(() => ({ result: AuthorizeResult.ALLOW, })); @@ -126,11 +131,12 @@ describe('AuthorizedSearchEngine', () => { beforeEach(() => { mockedQuery.mockReset(); mockedAuthorize.mockClear(); + mockedPermissionQuery.mockClear(); }); it('should forward the parameters correctly', async () => { mockedQuery.mockImplementation(async () => ({ results })); - mockedAuthorize.mockImplementation(allowAll); + const filters = { just: 1, a: 2, filter: 3 }; await authorizedSearchEngine.query( { term: 'term', filters, types: ['one', 'two'] }, @@ -148,7 +154,8 @@ describe('AuthorizedSearchEngine', () => { it('should forward the default types if none are passed', async () => { mockedQuery.mockImplementation(async () => ({ results })); - mockedAuthorize.mockImplementation(allowAll); + mockedPermissionQuery.mockImplementation(allowAll); + await authorizedSearchEngine.query({ term: '' }, options); expect(mockedQuery).toHaveBeenCalledWith( { term: '', types: ['users', 'templates', 'services', 'groups'] }, @@ -158,17 +165,17 @@ describe('AuthorizedSearchEngine', () => { it('should return all the results if all queries are allowed', async () => { mockedQuery.mockImplementation(async () => ({ results })); - mockedAuthorize.mockImplementation(allowAll); + mockedPermissionQuery.mockImplementation(allowAll); await expect( authorizedSearchEngine.query({ term: '' }, options), ).resolves.toEqual({ results }); - expect(mockedAuthorize).toHaveBeenCalledTimes(1); + expect(mockedPermissionQuery).toHaveBeenCalledTimes(1); }); it('should batch authorized requests', async () => { mockedQuery.mockImplementation(async () => ({ results })); - mockedAuthorize.mockImplementation(allowAll); + mockedPermissionQuery.mockImplementation(allowAll); await authorizedSearchEngine.query( { term: '', types: [typeUsers, typeTemplates] }, @@ -178,8 +185,8 @@ describe('AuthorizedSearchEngine', () => { { term: '', types: ['users', 'templates'] }, { token: 'token' }, ); - expect(mockedAuthorize).toHaveBeenCalledTimes(1); - expect(mockedAuthorize).toHaveBeenLastCalledWith( + expect(mockedPermissionQuery).toHaveBeenCalledTimes(1); + expect(mockedPermissionQuery).toHaveBeenLastCalledWith( [ { permission: defaultTypes[typeUsers].visibilityPermission }, { permission: defaultTypes[typeTemplates].visibilityPermission }, @@ -190,7 +197,7 @@ describe('AuthorizedSearchEngine', () => { it('should skip sending request for types that are not allowed', async () => { mockedQuery.mockImplementation(async () => ({ results })); - mockedAuthorize.mockImplementation(async queries => { + mockedPermissionQuery.mockImplementation(async queries => { return queries.map(query => { if ( query.permission.name === @@ -213,7 +220,7 @@ describe('AuthorizedSearchEngine', () => { { token: 'token' }, ); - expect(mockedAuthorize).toHaveBeenCalledTimes(1); + expect(mockedPermissionQuery).toHaveBeenCalledTimes(1); }); it('should perform result-by-result filtering', async () => { @@ -226,21 +233,12 @@ describe('AuthorizedSearchEngine', () => { results: resultsWithAuth, })); - const userToBeReturned = 8; - - mockedAuthorize.mockImplementation(async queries => + mockedPermissionQuery.mockImplementation(async queries => queries.map(query => { if ( query.permission.name === defaultTypes.users.visibilityPermission?.name ) { - if (query.resourceRef) { - return { - result: query.resourceRef.endsWith(userToBeReturned.toString()) - ? AuthorizeResult.ALLOW - : AuthorizeResult.DENY, - }; - } return { result: AuthorizeResult.CONDITIONAL, } as EvaluatePermissionResponse; @@ -252,9 +250,20 @@ describe('AuthorizedSearchEngine', () => { }), ); + mockedAuthorize.mockImplementation(async queries => + queries.map(query => { + return { + result: + query.resourceRef! === `users_doc_8` + ? AuthorizeResult.ALLOW + : AuthorizeResult.DENY, + }; + }), + ); + await expect( authorizedSearchEngine.query({ term: '' }, options), - ).resolves.toEqual({ results: [usersWithAuth[userToBeReturned]] }); + ).resolves.toEqual({ results: [usersWithAuth[8]] }); expect(mockedQuery).toHaveBeenCalledWith( { term: '', types: ['users'] }, @@ -284,27 +293,27 @@ describe('AuthorizedSearchEngine', () => { results: searchResults, })); - mockedAuthorize.mockImplementation(async queries => - queries.map(query => { - if (query.resourceRef) { - return { - result: AuthorizeResult.ALLOW, - }; - } + mockedPermissionQuery.mockImplementation(async queries => + queries.map( + _ => + ({ + result: AuthorizeResult.CONDITIONAL, + } as EvaluatePermissionResponse), + ), + ); - return { - result: AuthorizeResult.CONDITIONAL, - } as EvaluatePermissionResponse; - }), + mockedAuthorize.mockImplementation(async queries => + queries.map(_ => ({ + result: AuthorizeResult.ALLOW, + })), ); await expect( authorizedSearchEngine.query({ term: '', types: ['templates'] }, options), ).resolves.toEqual({ results: searchResults }); - expect(mockedAuthorize).toHaveBeenCalledTimes(2); - expect(mockedAuthorize).toHaveBeenNthCalledWith( - 1, + expect(mockedPermissionQuery).toHaveBeenCalledTimes(1); + expect(mockedPermissionQuery).toHaveBeenCalledWith( [ { permission: expect.objectContaining({ @@ -314,8 +323,8 @@ describe('AuthorizedSearchEngine', () => { ], { token: 'token' }, ); - expect(mockedAuthorize).toHaveBeenNthCalledWith( - 2, + expect(mockedAuthorize).toHaveBeenCalledTimes(1); + expect(mockedAuthorize).toHaveBeenCalledWith( [ { permission: expect.objectContaining({ @@ -329,7 +338,16 @@ describe('AuthorizedSearchEngine', () => { }); it('should perform search until the target number of results is reached', async () => { - mockedAuthorize.mockImplementation(async queries => + mockedPermissionQuery.mockImplementation(async queries => + queries.map( + _ => + ({ + result: AuthorizeResult.CONDITIONAL, + } as PolicyDecision), + ), + ); + + mockedPermissionQuery.mockImplementation(async queries => queries.map(query => { if (query.resourceRef) { return { @@ -342,6 +360,12 @@ describe('AuthorizedSearchEngine', () => { }), ); + mockedAuthorize.mockImplementation(async queries => + queries.map(_ => ({ + result: AuthorizeResult.ALLOW, + })), + ); + const usersWithAuth = generateSampleResults(typeUsers, true); const templatesWithAuth = generateSampleResults(typeTemplates, true); const servicesWithAuth = generateSampleResults(typeServices, true); @@ -405,20 +429,22 @@ describe('AuthorizedSearchEngine', () => { }); it('should perform search until the target number of results is reached, excluding unauthorized results', async () => { + mockedPermissionQuery.mockImplementation(async queries => + queries.map( + _ => + ({ + result: AuthorizeResult.CONDITIONAL, + } as PolicyDecision), + ), + ); + mockedAuthorize.mockImplementation(async queries => - queries.map(query => { - if (query.resourceRef) { - return { - result: - query.permission.name === 'search.services.read' - ? AuthorizeResult.DENY - : AuthorizeResult.ALLOW, - }; - } - return { - result: AuthorizeResult.CONDITIONAL, - } as EvaluatePermissionResponse; - }), + queries.map(query => ({ + result: + query.permission.name === 'search.services.read' + ? AuthorizeResult.DENY + : AuthorizeResult.ALLOW, + })), ); const usersWithAuth = generateSampleResults(typeUsers, true); @@ -494,15 +520,19 @@ describe('AuthorizedSearchEngine', () => { }); it('should discard results until the target cursor is reached', async () => { + mockedPermissionQuery.mockImplementation(async queries => + queries.map( + _ => + ({ + result: AuthorizeResult.CONDITIONAL, + } as PolicyDecision), + ), + ); + mockedAuthorize.mockImplementation(async queries => - queries.map(query => { - if (query.resourceRef) { - return { result: AuthorizeResult.ALLOW }; - } - return { - result: AuthorizeResult.CONDITIONAL, - } as EvaluatePermissionResponse; - }), + queries.map(_ => ({ + result: AuthorizeResult.ALLOW, + })), ); const usersWithAuth = generateSampleResults(typeUsers, true); diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts index a529bf04c6..a74cdf8eef 100644 --- a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts +++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts @@ -22,7 +22,9 @@ import { EvaluatePermissionRequest, AuthorizeResult, isResourcePermission, - PermissionAuthorizer, + PermissionEvaluator, + AuthorizePermissionRequest, + QueryPermissionRequest, } from '@backstage/plugin-permission-common'; import { DocumentTypeInfo, @@ -67,7 +69,7 @@ export class AuthorizedSearchEngine implements SearchEngine { constructor( private readonly searchEngine: SearchEngine, private readonly types: Record, - private readonly permissions: PermissionAuthorizer, + private readonly permissions: PermissionEvaluator, config: Config, ) { this.queryLatencyBudgetMs = @@ -89,8 +91,16 @@ export class AuthorizedSearchEngine implements SearchEngine { ): Promise { const queryStartTime = Date.now(); + const conditionFetcher = new DataLoader( + (requests: readonly QueryPermissionRequest[]) => + this.permissions.authorizeConditional(requests.slice(), options), + { + cacheKeyFn: ({ permission: { name } }) => name, + }, + ); + const authorizer = new DataLoader( - (requests: readonly EvaluatePermissionRequest[]) => + (requests: readonly AuthorizePermissionRequest[]) => this.permissions.authorize(requests.slice(), options), { // Serialize the permission name and resourceRef as @@ -100,6 +110,7 @@ export class AuthorizedSearchEngine implements SearchEngine { qs.stringify({ name, resourceRef }), }, ); + const requestedTypes = query.types || Object.keys(this.types); const typeDecisions = zipObject( @@ -108,9 +119,18 @@ export class AuthorizedSearchEngine implements SearchEngine { requestedTypes.map(type => { const permission = this.types[type]?.visibilityPermission; - return permission - ? authorizer.load({ permission }) - : { result: AuthorizeResult.ALLOW as const }; + // No permission configured for this document type - always allow. + if (!permission) { + return { result: AuthorizeResult.ALLOW as const }; + } + + // Resource permission supplied, so we need to check for conditional decisions. + if (isResourcePermission(permission)) { + return conditionFetcher.load({ permission }); + } + + // Non-resource permission supplied - we can perform a standard authorization. + return authorizer.load({ permission }); }), ), ); diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 2efab91256..26d901e519 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -16,7 +16,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; -import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { IndexBuilder, SearchEngine, @@ -26,10 +26,13 @@ import request from 'supertest'; import { createRouter } from './router'; -const mockPermissionAuthorizer: PermissionAuthorizer = { +const mockPermissionEvaluator: PermissionEvaluator = { authorize: () => { throw new Error('Not implemented'); }, + authorizeConditional: () => { + throw new Error('Not implemented'); + }, }; describe('createRouter', () => { @@ -59,7 +62,7 @@ describe('createRouter', () => { 'second-type': {}, }, config: new ConfigReader({ permissions: { enabled: false } }), - permissions: mockPermissionAuthorizer, + permissions: mockPermissionEvaluator, logger, }); app = express().use(router); @@ -164,7 +167,7 @@ describe('createRouter', () => { engine: indexBuilder.getSearchEngine(), types: indexBuilder.getDocumentTypes(), config: new ConfigReader({ permissions: { enabled: false } }), - permissions: mockPermissionAuthorizer, + permissions: mockPermissionEvaluator, logger, }); app = express().use(router); diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index ff91465cf4..11f0eb22e6 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -23,7 +23,11 @@ import { InputError } from '@backstage/errors'; import { Config } from '@backstage/config'; import { JsonObject, JsonValue } from '@backstage/types'; import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; -import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; +import { + PermissionAuthorizer, + PermissionEvaluator, + toPermissionEvaluator, +} from '@backstage/plugin-permission-common'; import { DocumentTypeInfo, IndexableResultSet, @@ -50,7 +54,7 @@ const jsonObjectSchema: z.ZodSchema = z.lazy(() => { export type RouterOptions = { engine: SearchEngine; types: Record; - permissions: PermissionAuthorizer; + permissions: PermissionEvaluator | PermissionAuthorizer; config: Config; logger: Logger; }; @@ -71,8 +75,23 @@ export async function createRouter( pageCursor: z.string().optional(), }); + let permissionEvaluator: PermissionEvaluator; + if ('query' in permissions) { + permissionEvaluator = permissions as PermissionEvaluator; + } else { + logger.warn( + 'PermissionAuthorizer is deprecated. Please use an instance of PermissionEvaluator instead of PermissionAuthorizer in PluginEnvironment#permissions', + ); + permissionEvaluator = toPermissionEvaluator(permissions); + } + const engine = config.getOptionalBoolean('permission.enabled') - ? new AuthorizedSearchEngine(inputEngine, types, permissions, config) + ? new AuthorizedSearchEngine( + inputEngine, + types, + permissionEvaluator, + config, + ) : inputEngine; const filterResultSet = ({ results, ...resultSet }: SearchResultSet) => ({