From e43290ce96b52e4a09267468282faaadcf5603f9 Mon Sep 17 00:00:00 2001 From: Joe Porpeglia Date: Tue, 22 Mar 2022 10:24:08 -0400 Subject: [PATCH] Rename permission backend request and response types Signed-off-by: Joe Porpeglia --- .../apis/PermissionApi/MockPermissionApi.ts | 10 +++-- .../permission-backend/src/service/router.ts | 40 ++++++++++--------- .../src/PermissionClient.test.ts | 10 ++--- .../permission-common/src/PermissionClient.ts | 20 +++++----- plugins/permission-common/src/types/api.ts | 40 ++++++++++--------- plugins/permission-common/src/types/index.ts | 8 ++-- .../permission-common/src/types/permission.ts | 6 +-- .../src/ServerPermissionClient.test.ts | 4 +- .../src/ServerPermissionClient.ts | 12 +++--- plugins/permission-node/src/policy/types.ts | 7 +++- .../src/apis/IdentityPermissionApi.ts | 8 ++-- .../src/apis/PermissionApi.ts | 8 ++-- .../service/AuthorizedSearchEngine.test.ts | 18 ++++++--- .../src/service/AuthorizedSearchEngine.ts | 13 +++--- 14 files changed, 114 insertions(+), 90 deletions(-) diff --git a/packages/test-utils/src/testUtils/apis/PermissionApi/MockPermissionApi.ts b/packages/test-utils/src/testUtils/apis/PermissionApi/MockPermissionApi.ts index ac5c44248d..163af5dd53 100644 --- a/packages/test-utils/src/testUtils/apis/PermissionApi/MockPermissionApi.ts +++ b/packages/test-utils/src/testUtils/apis/PermissionApi/MockPermissionApi.ts @@ -16,8 +16,8 @@ import { PermissionApi } from '@backstage/plugin-permission-react'; import { - AuthorizeDecision, - AuthorizeQuery, + EvaluatePermissionResponse, + EvaluatePermissionRequest, AuthorizeResult, } from '@backstage/plugin-permission-common'; @@ -31,12 +31,14 @@ import { export class MockPermissionApi implements PermissionApi { constructor( private readonly requestHandler: ( - request: AuthorizeQuery, + request: EvaluatePermissionRequest, ) => AuthorizeResult.ALLOW | AuthorizeResult.DENY = () => AuthorizeResult.ALLOW, ) {} - async authorize(request: AuthorizeQuery): Promise { + async authorize( + request: EvaluatePermissionRequest, + ): Promise { return { result: this.requestHandler(request) }; } } diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index ca5467fa6e..ea7fbcca24 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -30,11 +30,11 @@ import { } from '@backstage/plugin-auth-node'; import { AuthorizeResult, - AuthorizeDecision, - AuthorizeQuery, + EvaluatePermissionResponse, + EvaluatePermissionRequest, IdentifiedPermissionMessage, - AuthorizeRequest, - AuthorizeResponse, + EvaluatePermissionRequestBatch, + EvaluatePermissionResponseBatch, isResourcePermission, PermissionAttributes, } from '@backstage/plugin-permission-common'; @@ -73,17 +73,19 @@ const permissionSchema = z.union([ }), ]); -const querySchema: z.ZodSchema> = - z.object({ - id: z.string(), - resourceRef: z.string().optional(), - permission: permissionSchema, - }); - -const requestSchema: z.ZodSchema = z.object({ - items: z.array(querySchema), +const evaluatePermissionRequestSchema: z.ZodSchema< + IdentifiedPermissionMessage +> = z.object({ + id: z.string(), + resourceRef: z.string().optional(), + permission: permissionSchema, }); +const evaluatePermissionRequestBatchSchema: z.ZodSchema = + z.object({ + items: z.array(evaluatePermissionRequestSchema), + }); + /** * Options required when constructing a new {@link express#Router} using * {@link createRouter}. @@ -99,12 +101,12 @@ export interface RouterOptions { } const handleRequest = async ( - requests: IdentifiedPermissionMessage[], + requests: IdentifiedPermissionMessage[], user: BackstageIdentityResponse | undefined, policy: PermissionPolicy, permissionIntegrationClient: PermissionIntegrationClient, authHeader?: string, -): Promise[]> => { +): Promise[]> => { const applyConditionsLoaderFor = memoize((pluginId: string) => { return new DataLoader< ApplyConditionsRequestEntry, @@ -184,15 +186,17 @@ export async function createRouter( router.post( '/authorize', async ( - req: Request, - res: Response, + req: Request, + res: Response, ) => { const token = getBearerTokenFromAuthorizationHeader( req.header('authorization'), ); const user = token ? await identity.authenticate(token) : undefined; - const parseResult = requestSchema.safeParse(req.body); + const parseResult = evaluatePermissionRequestBatchSchema.safeParse( + req.body, + ); if (!parseResult.success) { throw new InputError(parseResult.error.toString()); diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 36f26d883f..65a7c7fbbe 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -19,7 +19,7 @@ import { setupServer } from 'msw/node'; import { ConfigReader } from '@backstage/config'; import { PermissionClient } from './PermissionClient'; import { - AuthorizeQuery, + EvaluatePermissionRequest, AuthorizeResult, IdentifiedPermissionMessage, } from './types/api'; @@ -59,7 +59,7 @@ describe('PermissionClient', () => { const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { const responses = req.body.items.map( - (a: IdentifiedPermissionMessage) => ({ + (a: IdentifiedPermissionMessage) => ({ id: a.id, result: AuthorizeResult.ALLOW, }), @@ -147,7 +147,7 @@ describe('PermissionClient', () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { const responses = req.body.items.map( - (a: IdentifiedPermissionMessage) => ({ + (a: IdentifiedPermissionMessage) => ({ id: a.id, outcome: AuthorizeResult.ALLOW, }), @@ -165,7 +165,7 @@ describe('PermissionClient', () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { const responses = req.body.map( - (a: IdentifiedPermissionMessage) => ({ + (a: IdentifiedPermissionMessage) => ({ id: a.id, result: AuthorizeResult.DENY, }), @@ -189,7 +189,7 @@ describe('PermissionClient', () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { const responses = req.body.map( - (a: IdentifiedPermissionMessage) => ({ + (a: IdentifiedPermissionMessage) => ({ id: a.id, outcome: AuthorizeResult.DENY, }), diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index fbae34cc63..3027234b72 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -21,13 +21,13 @@ import * as uuid from 'uuid'; import { z } from 'zod'; import { AuthorizeResult, - AuthorizeQuery, - AuthorizeDecision, + EvaluatePermissionRequest, + EvaluatePermissionResponse, IdentifiedPermissionMessage, PermissionCriteria, PermissionCondition, - AuthorizeResponse, - AuthorizeRequest, + EvaluatePermissionResponseBatch, + EvaluatePermissionRequestBatch, } from './types/api'; import { DiscoveryApi } from './types/discovery'; import { @@ -107,9 +107,9 @@ export class PermissionClient implements PermissionAuthorizer { * @public */ async authorize( - queries: AuthorizeQuery[], + queries: EvaluatePermissionRequest[], options?: AuthorizeRequestOptions, - ): Promise { + ): 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 @@ -119,7 +119,7 @@ export class PermissionClient implements PermissionAuthorizer { return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); } - const request: AuthorizeRequest = { + const request: EvaluatePermissionRequestBatch = { items: queries.map(query => ({ id: uuid.v4(), ...query, @@ -145,7 +145,7 @@ export class PermissionClient implements PermissionAuthorizer { const responsesById = responseBody.items.reduce((acc, r) => { acc[r.id] = r; return acc; - }, {} as Record>); + }, {} as Record>); return request.items.map(query => responsesById[query.id]); } @@ -155,9 +155,9 @@ export class PermissionClient implements PermissionAuthorizer { } private assertValidResponse( - request: AuthorizeRequest, + request: EvaluatePermissionRequestBatch, json: any, - ): asserts json is AuthorizeResponse { + ): asserts json is EvaluatePermissionResponseBatch { const authorizedResponses = responseSchema.parse(json); const responseIds = authorizedResponses.items.map(r => r.id); const hasAllRequestIds = request.items.every(r => diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index 97b1182805..b0bcf94f56 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -90,21 +90,6 @@ export type PolicyDecision = | DefinitivePolicyDecision | ConditionalPolicyDecision; -/** - * An individual authorization request for {@link PermissionClient#authorize}. - * @public - */ -export type AuthorizeQuery = { - permission: Permission; - resourceRef?: string; -}; - -/** - * A batch of authorization requests from {@link PermissionClient#authorize}. - * @public - */ -export type AuthorizeRequest = PermissionMessageBatch; - /** * A condition returned with a CONDITIONAL authorization response. * @@ -159,10 +144,26 @@ export type PermissionCriteria = | TQuery; /** - * An individual authorization response from {@link PermissionClient#authorize}. + * An individual request sent to the permission backend. * @public */ -export type AuthorizeDecision = +export type EvaluatePermissionRequest = { + permission: Permission; + resourceRef?: string; +}; + +/** + * A batch of requests sent to the permission backend. + * @public + */ +export type EvaluatePermissionRequestBatch = + PermissionMessageBatch; + +/** + * An individual response from the permission backend. + * @public + */ +export type EvaluatePermissionResponse = | { result: AuthorizeResult.ALLOW | AuthorizeResult.DENY } | { result: AuthorizeResult.CONDITIONAL; @@ -170,7 +171,8 @@ export type AuthorizeDecision = }; /** - * A batch of authorization responses from {@link PermissionClient#authorize}. + * A batch of responses the permission backend. * @public */ -export type AuthorizeResponse = PermissionMessageBatch; +export type EvaluatePermissionResponseBatch = + PermissionMessageBatch; diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index f89f0c8aec..1a993a981c 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -16,10 +16,10 @@ export { AuthorizeResult } from './api'; export type { - AuthorizeQuery, - AuthorizeRequest, - AuthorizeDecision, - AuthorizeResponse, + EvaluatePermissionRequest, + EvaluatePermissionRequestBatch, + EvaluatePermissionResponse, + EvaluatePermissionResponseBatch, IdentifiedPermissionMessage, PermissionMessageBatch, ConditionalPolicyDecision, diff --git a/plugins/permission-common/src/types/permission.ts b/plugins/permission-common/src/types/permission.ts index ebfdadb48b..34e7884793 100644 --- a/plugins/permission-common/src/types/permission.ts +++ b/plugins/permission-common/src/types/permission.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { AuthorizeQuery, AuthorizeDecision } from './api'; +import { EvaluatePermissionRequest, EvaluatePermissionResponse } from './api'; /** * The attributes related to a given permission; these should be generic and widely applicable to @@ -94,9 +94,9 @@ export type ResourcePermission = */ export interface PermissionAuthorizer { authorize( - queries: AuthorizeQuery[], + requests: EvaluatePermissionRequest[], options?: AuthorizeRequestOptions, - ): Promise; + ): Promise; } /** diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index 842f996a6a..f5711ed4a0 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -17,7 +17,7 @@ import { ServerPermissionClient } from './ServerPermissionClient'; import { IdentifiedPermissionMessage, - AuthorizeQuery, + EvaluatePermissionRequest, AuthorizeResult, createPermission, } from '@backstage/plugin-permission-common'; @@ -33,7 +33,7 @@ import { RestContext, rest } from 'msw'; const server = setupServer(); const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { const responses = req.body.items.map( - (r: IdentifiedPermissionMessage) => ({ + (r: IdentifiedPermissionMessage) => ({ id: r.id, result: AuthorizeResult.ALLOW, }), diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index 8c03016268..56381dff17 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -20,9 +20,9 @@ import { } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import { - AuthorizeQuery, + EvaluatePermissionRequest, AuthorizeRequestOptions, - AuthorizeDecision, + EvaluatePermissionResponse, AuthorizeResult, PermissionClient, PermissionAuthorizer, @@ -78,9 +78,9 @@ export class ServerPermissionClient implements PermissionAuthorizer { } async authorize( - queries: AuthorizeQuery[], + requests: EvaluatePermissionRequest[], options?: AuthorizeRequestOptions, - ): Promise { + ): 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 @@ -89,9 +89,9 @@ export class ServerPermissionClient implements PermissionAuthorizer { !this.permissionEnabled || (await this.isValidServerToken(options?.token)) ) { - return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); + return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); } - return this.permissionClient.authorize(queries, options); + return this.permissionClient.authorize(requests, options); } private async isValidServerToken( diff --git a/plugins/permission-node/src/policy/types.ts b/plugins/permission-node/src/policy/types.ts index fed803a2a9..36145eea77 100644 --- a/plugins/permission-node/src/policy/types.ts +++ b/plugins/permission-node/src/policy/types.ts @@ -15,7 +15,7 @@ */ import { - AuthorizeQuery, + EvaluatePermissionRequest, PolicyDecision, } from '@backstage/plugin-permission-common'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; @@ -31,7 +31,10 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; * * @public */ -export type PolicyAuthorizeQuery = Omit; +export type PolicyAuthorizeQuery = Omit< + EvaluatePermissionRequest, + 'resourceRef' +>; /** * A policy to evaluate authorization requests for any permissioned action performed in Backstage. diff --git a/plugins/permission-react/src/apis/IdentityPermissionApi.ts b/plugins/permission-react/src/apis/IdentityPermissionApi.ts index 6e5d54f61d..80d4eb8859 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 { - AuthorizeQuery, - AuthorizeDecision, + EvaluatePermissionRequest, + EvaluatePermissionResponse, PermissionClient, } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; @@ -44,7 +44,9 @@ export class IdentityPermissionApi implements PermissionApi { return new IdentityPermissionApi(permissionClient, identity); } - async authorize(request: AuthorizeQuery): Promise { + async authorize( + request: EvaluatePermissionRequest, + ): Promise { const response = await this.permissionClient.authorize( [request], await this.identityApi.getCredentials(), diff --git a/plugins/permission-react/src/apis/PermissionApi.ts b/plugins/permission-react/src/apis/PermissionApi.ts index 69a42cae91..d3a37421ef 100644 --- a/plugins/permission-react/src/apis/PermissionApi.ts +++ b/plugins/permission-react/src/apis/PermissionApi.ts @@ -15,8 +15,8 @@ */ import { - AuthorizeQuery, - AuthorizeDecision, + EvaluatePermissionRequest, + EvaluatePermissionResponse, } from '@backstage/plugin-permission-common'; import { ApiRef, createApiRef } from '@backstage/core-plugin-api'; @@ -27,7 +27,9 @@ import { ApiRef, createApiRef } from '@backstage/core-plugin-api'; * @public */ export type PermissionApi = { - authorize(request: AuthorizeQuery): Promise; + authorize( + request: EvaluatePermissionRequest, + ): Promise; }; /** diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts index 4198730947..47503312fb 100644 --- a/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts +++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts @@ -16,7 +16,7 @@ import { ConfigReader } from '@backstage/config'; import { - AuthorizeDecision, + EvaluatePermissionResponse, AuthorizeResult, createPermission, PermissionAuthorizer, @@ -243,7 +243,7 @@ describe('AuthorizedSearchEngine', () => { } return { result: AuthorizeResult.CONDITIONAL, - } as AuthorizeDecision; + } as EvaluatePermissionResponse; } return { @@ -294,7 +294,7 @@ describe('AuthorizedSearchEngine', () => { return { result: AuthorizeResult.CONDITIONAL, - } as AuthorizeDecision; + } as EvaluatePermissionResponse; }), ); @@ -336,7 +336,9 @@ describe('AuthorizedSearchEngine', () => { result: AuthorizeResult.ALLOW, }; } - return { result: AuthorizeResult.CONDITIONAL } as AuthorizeDecision; + return { + result: AuthorizeResult.CONDITIONAL, + } as EvaluatePermissionResponse; }), ); @@ -413,7 +415,9 @@ describe('AuthorizedSearchEngine', () => { : AuthorizeResult.ALLOW, }; } - return { result: AuthorizeResult.CONDITIONAL } as AuthorizeDecision; + return { + result: AuthorizeResult.CONDITIONAL, + } as EvaluatePermissionResponse; }), ); @@ -495,7 +499,9 @@ describe('AuthorizedSearchEngine', () => { if (query.resourceRef) { return { result: AuthorizeResult.ALLOW }; } - return { result: AuthorizeResult.CONDITIONAL } as AuthorizeDecision; + return { + result: AuthorizeResult.CONDITIONAL, + } as EvaluatePermissionResponse; }), ); diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts index 3e7dc6e147..a529bf04c6 100644 --- a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts +++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts @@ -18,8 +18,8 @@ import { compact, zipObject } from 'lodash'; import qs from 'qs'; import DataLoader from 'dataloader'; import { - AuthorizeDecision, - AuthorizeQuery, + EvaluatePermissionResponse, + EvaluatePermissionRequest, AuthorizeResult, isResourcePermission, PermissionAuthorizer, @@ -90,7 +90,7 @@ export class AuthorizedSearchEngine implements SearchEngine { const queryStartTime = Date.now(); const authorizer = new DataLoader( - (requests: readonly AuthorizeQuery[]) => + (requests: readonly EvaluatePermissionRequest[]) => this.permissions.authorize(requests.slice(), options), { // Serialize the permission name and resourceRef as @@ -185,8 +185,11 @@ export class AuthorizedSearchEngine implements SearchEngine { private async filterResults( results: IndexableResult[], - typeDecisions: Record, - authorizer: DataLoader, + typeDecisions: Record, + authorizer: DataLoader< + EvaluatePermissionRequest, + EvaluatePermissionResponse + >, ) { return compact( await Promise.all(