diff --git a/.changeset/real-geckos-work.md b/.changeset/real-geckos-work.md new file mode 100644 index 0000000000000..a62b18e50fa85 --- /dev/null +++ b/.changeset/real-geckos-work.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-common': patch +--- + +Create PermissionAuthorizer interface for PermissionClients diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index e56a27b81f4b3..3500afe32a1d8 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -304,11 +304,11 @@ export const apis = [ createApiFactory({ api: permissionApiRef, deps: { - discoveryApi: discoveryApiRef, - identityApi: identityApiRef, - configApi: configApiRef, + discovery: discoveryApiRef, + identity: identityApiRef, + config: configApiRef, }, - factory: ({ configApi, discoveryApi, identityApi }) => - IdentityPermissionApi.create({ configApi, discoveryApi, identityApi }), + factory: ({ config, discovery, identity }) => + IdentityPermissionApi.create({ config, discovery, identity }), }), ]; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 96def8d03f3f5..c39865058830d 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -64,10 +64,10 @@ function makeCreateEnv(config: Config) { const reader = UrlReaders.default({ logger: root, config }); const discovery = SingleHostDiscovery.fromConfig(config); const tokenManager = ServerTokenManager.fromConfig(config, { logger: root }); - const permissions = new ServerPermissionClient({ - discoveryApi: discovery, - configApi: config, - serverTokenManager: tokenManager, + const permissions = ServerPermissionClient.create({ + discovery, + config, + tokenManager, }); root.info(`Created UrlReader ${reader}`); diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md index cc2a433504ce9..70795d14ac3e3 100644 --- a/plugins/permission-common/api-report.md +++ b/plugins/permission-common/api-report.md @@ -68,16 +68,21 @@ export type PermissionAttributes = { }; // @public -export class PermissionClient { - constructor(options: { discoveryApi: DiscoveryApi; configApi: Config }); +export interface PermissionAuthorizer { + // (undocumented) + authorize( + requests: AuthorizeRequest[], + options?: AuthorizeRequestOptions, + ): Promise; +} + +// @public +export class PermissionClient implements PermissionAuthorizer { + constructor(options: { discovery: DiscoveryApi; config: Config }); authorize( requests: AuthorizeRequest[], options?: AuthorizeRequestOptions, ): Promise; - // (undocumented) - protected readonly enabled: boolean; - // (undocumented) - protected shouldBypass(_options?: AuthorizeRequestOptions): Promise; } // @public diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index b2e66986a123c..a432735a5e8c9 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -26,14 +26,14 @@ const server = setupServer(); const token = 'fake-token'; const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; -const discoveryApi: DiscoveryApi = { +const discovery: DiscoveryApi = { async getBaseUrl() { return mockBaseUrl; }, }; const client: PermissionClient = new PermissionClient({ - discoveryApi, - configApi: new ConfigReader({ permission: { enabled: true } }), + discovery, + config: new ConfigReader({ permission: { enabled: true } }), }); const mockPermission: Permission = { @@ -158,8 +158,8 @@ describe('PermissionClient', () => { }, ); const disabled = new PermissionClient({ - discoveryApi, - configApi: new ConfigReader({ permission: { enabled: false } }), + discovery, + config: new ConfigReader({ permission: { enabled: false } }), }); const response = await disabled.authorize([mockAuthorizeRequest]); expect(response[0]).toEqual( @@ -180,8 +180,8 @@ describe('PermissionClient', () => { }, ); const disabled = new PermissionClient({ - discoveryApi, - configApi: new ConfigReader({}), + discovery, + config: new ConfigReader({}), }); const response = await disabled.authorize([mockAuthorizeRequest]); expect(response[0]).toEqual( diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index c61a9cadd9fbf..62e5ec817212c 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -29,7 +29,7 @@ import { } from './types/api'; import { DiscoveryApi } from './types/discovery'; import { - PermissionClientInterface, + PermissionAuthorizer, AuthorizeRequestOptions, } from './types/permission'; @@ -67,14 +67,14 @@ const responseSchema = z.array( * An isomorphic client for requesting authorization for Backstage permissions. * @public */ -export class PermissionClient implements PermissionClientInterface { +export class PermissionClient implements PermissionAuthorizer { private readonly enabled: boolean; - private readonly discoveryApi: DiscoveryApi; + private readonly discovery: DiscoveryApi; - constructor(options: { discoveryApi: DiscoveryApi; configApi: Config }) { - this.discoveryApi = options.discoveryApi; + constructor(options: { discovery: DiscoveryApi; config: Config }) { + this.discovery = options.discovery; this.enabled = - options.configApi.getOptionalBoolean('permission.enabled') ?? false; + options.config.getOptionalBoolean('permission.enabled') ?? false; } /** @@ -113,7 +113,7 @@ export class PermissionClient implements PermissionClientInterface { }), ); - const permissionApi = await this.discoveryApi.getBaseUrl('permission'); + const permissionApi = await this.discovery.getBaseUrl('permission'); const response = await fetch(`${permissionApi}/authorize`, { method: 'POST', body: JSON.stringify(identifiedRequests), diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index 4d28cca7b5173..583a3577bd11a 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -26,6 +26,6 @@ export type { DiscoveryApi } from './discovery'; export type { PermissionAttributes, Permission, - PermissionClientInterface, + PermissionAuthorizer, AuthorizeRequestOptions, } from './permission'; diff --git a/plugins/permission-common/src/types/permission.ts b/plugins/permission-common/src/types/permission.ts index a9c82d6a2019c..246f1bec72a55 100644 --- a/plugins/permission-common/src/types/permission.ts +++ b/plugins/permission-common/src/types/permission.ts @@ -42,7 +42,11 @@ export type Permission = { resourceType?: string; }; -export interface PermissionClientInterface { +/** + * A client interacting with the permission backend can implement this authorizer interface. + * @public + */ +export interface PermissionAuthorizer { authorize( requests: AuthorizeRequest[], options?: AuthorizeRequestOptions, @@ -50,7 +54,7 @@ export interface PermissionClientInterface { } /** - * Options for authorization requests; currently only an optional auth token. + * Options for authorization requests. * @public */ export type AuthorizeRequestOptions = { diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 90181925c402a..f41350231f0b7 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -5,13 +5,14 @@ ```ts import { AuthorizeRequest } from '@backstage/plugin-permission-common'; import { AuthorizeRequestOptions } from '@backstage/plugin-permission-common'; +import { AuthorizeResponse } from '@backstage/plugin-permission-common'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-backend'; import { Config } from '@backstage/config'; -import { DiscoveryApi } from '@backstage/plugin-permission-common'; -import { PermissionClient } 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 { PluginEndpointDiscovery } from '@backstage/backend-common'; import { Router } from 'express'; import { TokenManager } from '@backstage/backend-common'; @@ -126,13 +127,17 @@ export type PolicyDecision = | ConditionalPolicyDecision; // @public -export class ServerPermissionClient extends PermissionClient { - constructor(options: { - discoveryApi: DiscoveryApi; - configApi: Config; - serverTokenManager: TokenManager; - }); +export class ServerPermissionClient implements PermissionAuthorizer { // (undocumented) - shouldBypass(options?: AuthorizeRequestOptions): Promise; + authorize( + requests: AuthorizeRequest[], + options?: AuthorizeRequestOptions, + ): Promise; + // (undocumented) + static create(options: { + discovery: PluginEndpointDiscovery; + config: Config; + tokenManager: TokenManager; + }): ServerPermissionClient; } ``` diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index 8efd6414a7d5b..5965247c1699f 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -16,14 +16,17 @@ import { ServerPermissionClient } from '.'; import { - DiscoveryApi, Permission, Identified, AuthorizeRequest, AuthorizeResult, } from '@backstage/plugin-permission-common'; import { ConfigReader } from '@backstage/config'; -import { getVoidLogger, ServerTokenManager } from '@backstage/backend-common'; +import { + getVoidLogger, + PluginEndpointDiscovery, + ServerTokenManager, +} from '@backstage/backend-common'; import { setupServer } from 'msw/node'; import { RestContext, rest } from 'msw'; @@ -37,10 +40,13 @@ const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { return res(json(responses)); }); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; -const discoveryApi: DiscoveryApi = { +const discovery: PluginEndpointDiscovery = { async getBaseUrl() { return mockBaseUrl; }, + async getExternalBaseUrl() { + return mockBaseUrl; + }, }; const testPermission: Permission = { name: 'test.permission', @@ -62,10 +68,10 @@ describe('ServerPermissionClient', () => { afterEach(() => server.resetHandlers()); it('should bypass authorization if permissions are disabled', async () => { - const client = new ServerPermissionClient({ - discoveryApi, - configApi: new ConfigReader({}), - serverTokenManager: ServerTokenManager.noop(), + const client = ServerPermissionClient.create({ + discovery, + config: new ConfigReader({}), + tokenManager: ServerTokenManager.noop(), }); await client.authorize([{ permission: testPermission }]); @@ -75,10 +81,10 @@ describe('ServerPermissionClient', () => { it('should bypass authorization if permissions are enabled and request has valid server token', async () => { const tokenManager = ServerTokenManager.fromConfig(config, { logger }); - const client = new ServerPermissionClient({ - discoveryApi, - configApi: config, - serverTokenManager: tokenManager, + const client = ServerPermissionClient.create({ + discovery, + config, + tokenManager, }); await client.authorize([{ permission: testPermission }], { @@ -90,10 +96,10 @@ describe('ServerPermissionClient', () => { 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 = new ServerPermissionClient({ - discoveryApi, - configApi: config, - serverTokenManager: tokenManager, + const client = ServerPermissionClient.create({ + discovery, + config, + tokenManager, }); await client.authorize([{ permission: testPermission }], { @@ -104,13 +110,12 @@ describe('ServerPermissionClient', () => { }); it('should error if permissions are enabled but a no-op token manager is configured', async () => { - expect( - () => - new ServerPermissionClient({ - discoveryApi, - configApi: config, - serverTokenManager: ServerTokenManager.noop(), - }), + expect(() => + ServerPermissionClient.create({ + discovery, + config, + tokenManager: ServerTokenManager.noop(), + }), ).toThrowError( 'You must configure at least one key in backend.auth.keys if permissions are enabled.', ); diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index 734ecd73bb46f..51c9bcf04fec6 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -14,16 +14,19 @@ * limitations under the License. */ -import { TokenManager, ServerTokenManager } from '@backstage/backend-common'; +import { + TokenManager, + ServerTokenManager, + PluginEndpointDiscovery, +} from '@backstage/backend-common'; import { Config } from '@backstage/config'; import { AuthorizeRequest, AuthorizeRequestOptions, AuthorizeResponse, AuthorizeResult, - DiscoveryApi, PermissionClient, - PermissionClientInterface, + PermissionAuthorizer, } from '@backstage/plugin-permission-common'; /** @@ -32,32 +35,47 @@ import { * backend-to-backend requests. * @public */ -export class ServerPermissionClient implements PermissionClientInterface { - private readonly serverTokenManager: TokenManager; +export class ServerPermissionClient implements PermissionAuthorizer { private readonly permissionClient: PermissionClient; + private readonly tokenManager: TokenManager; private readonly permissionEnabled: boolean; - constructor(options: { - discoveryApi: DiscoveryApi; - configApi: Config; - serverTokenManager: TokenManager; + static create(options: { + discovery: PluginEndpointDiscovery; + config: Config; + tokenManager: TokenManager; }) { - const { discoveryApi, configApi, serverTokenManager } = options; - this.permissionClient = new PermissionClient({ discoveryApi, configApi }); - this.permissionEnabled = - options.configApi.getOptionalBoolean('permission.enabled') ?? false; + const { discovery, config, tokenManager } = options; + const permissionClient = new PermissionClient({ discovery, config }); + const permissionEnabled = + config.getOptionalBoolean('permission.enabled') ?? false; if ( - this.permissionEnabled && + permissionEnabled && // TODO: Find a cleaner way of ensuring usage of SERVER token manager when // permissions are enabled. - serverTokenManager instanceof ServerTokenManager.noop().constructor + tokenManager instanceof ServerTokenManager.noop().constructor ) { throw new Error( 'You must configure at least one key in backend.auth.keys if permissions are enabled.', ); } - this.serverTokenManager = serverTokenManager; + + return new ServerPermissionClient({ + permissionClient, + tokenManager, + permissionEnabled, + }); + } + + private constructor(options: { + permissionClient: PermissionClient; + tokenManager: TokenManager; + permissionEnabled: boolean; + }) { + this.permissionClient = options.permissionClient; + this.tokenManager = options.tokenManager; + this.permissionEnabled = options.permissionEnabled; } async authorize( @@ -83,7 +101,7 @@ export class ServerPermissionClient implements PermissionClientInterface { if (!token) { return false; } - return this.serverTokenManager + return this.tokenManager .authenticate(token) .then(() => true) .catch(() => false); diff --git a/plugins/permission-react/api-report.md b/plugins/permission-react/api-report.md index c10317495a029..799b3ad54f5d1 100644 --- a/plugins/permission-react/api-report.md +++ b/plugins/permission-react/api-report.md @@ -27,9 +27,9 @@ export class IdentityPermissionApi implements PermissionApi { authorize(request: AuthorizeRequest): Promise; // (undocumented) static create(options: { - configApi: Config; - discoveryApi: DiscoveryApi; - identityApi: IdentityApi; + config: Config; + discovery: DiscoveryApi; + identity: IdentityApi; }): IdentityPermissionApi; } diff --git a/plugins/permission-react/src/apis/IdentityPermissionApi.ts b/plugins/permission-react/src/apis/IdentityPermissionApi.ts index 19de564f84751..818eca17fa707 100644 --- a/plugins/permission-react/src/apis/IdentityPermissionApi.ts +++ b/plugins/permission-react/src/apis/IdentityPermissionApi.ts @@ -35,13 +35,13 @@ export class IdentityPermissionApi implements PermissionApi { ) {} static create(options: { - configApi: Config; - discoveryApi: DiscoveryApi; - identityApi: IdentityApi; + config: Config; + discovery: DiscoveryApi; + identity: IdentityApi; }) { - const { configApi, discoveryApi, identityApi } = options; - const permissionClient = new PermissionClient({ discoveryApi, configApi }); - return new IdentityPermissionApi(permissionClient, identityApi); + const { config, discovery, identity } = options; + const permissionClient = new PermissionClient({ discovery, config }); + return new IdentityPermissionApi(permissionClient, identity); } async authorize(request: AuthorizeRequest): Promise {