diff --git a/.changeset/gold-worms-complain.md b/.changeset/gold-worms-complain.md new file mode 100644 index 0000000000..229293cc2f --- /dev/null +++ b/.changeset/gold-worms-complain.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': minor +--- + +**BREAKING**: Updated the `ServerPermissionClient` to match the new `PermissionsService` interface, where the deprecated `token` option has been removed and the options are now required. diff --git a/.changeset/swift-seals-drive.md b/.changeset/swift-seals-drive.md new file mode 100644 index 0000000000..a03f490d3f --- /dev/null +++ b/.changeset/swift-seals-drive.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': minor +--- + +**BREAKING**: The `PermissionsService` no longer supports passing the deprecated `token` option, and the request options are now required. diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 7b1e6ef2d3..1937ba6ca9 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -9,6 +9,7 @@ import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common' import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; import { Duration } from 'luxon'; +import { EvaluatorRequestOptions } from '@backstage/plugin-permission-common'; import { Handler } from 'express'; import { HumanDuration } from '@backstage/types'; import { IdentityApi } from '@backstage/plugin-auth-node'; @@ -423,22 +424,20 @@ export interface LoggerService { export interface PermissionsService extends PermissionEvaluator { authorize( requests: AuthorizePermissionRequest[], - options?: PermissionsServiceRequestOptions, + options: PermissionsServiceRequestOptions, ): Promise; authorizeConditional( requests: QueryPermissionRequest[], - options?: PermissionsServiceRequestOptions, + options: PermissionsServiceRequestOptions, ): Promise; } // @public -export type PermissionsServiceRequestOptions = - | { - token?: string; - } - | { - credentials: BackstageCredentials; - }; +export interface PermissionsServiceRequestOptions + extends EvaluatorRequestOptions { + // (undocumented) + credentials: BackstageCredentials; +} // @public export interface PluginMetadataService { diff --git a/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts b/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts index c5046a988b..a25081d17a 100644 --- a/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts +++ b/packages/backend-plugin-api/src/services/definitions/PermissionsService.ts @@ -17,6 +17,7 @@ import { AuthorizePermissionRequest, AuthorizePermissionResponse, + EvaluatorRequestOptions, PermissionEvaluator, QueryPermissionRequest, QueryPermissionResponse, @@ -24,18 +25,14 @@ import { import { BackstageCredentials } from './AuthService'; /** - * Options for {@link @backstage/plugin-permission-common#PermissionEvaluator} requests. + * Options for {@link PermissionsService} requests. * * @public */ -export type PermissionsServiceRequestOptions = - | { - /** @deprecated use the `credentials` option instead. */ - token?: string; - } - | { - credentials: BackstageCredentials; - }; +export interface PermissionsServiceRequestOptions + extends EvaluatorRequestOptions { + credentials: BackstageCredentials; +} /** * Permission system integration for authorization of user/service actions. @@ -59,7 +56,7 @@ export interface PermissionsService extends PermissionEvaluator { */ authorize( requests: AuthorizePermissionRequest[], - options?: PermissionsServiceRequestOptions, + options: PermissionsServiceRequestOptions, ): Promise; /** @@ -79,6 +76,6 @@ export interface PermissionsService extends PermissionEvaluator { */ authorizeConditional( requests: QueryPermissionRequest[], - options?: PermissionsServiceRequestOptions, + options: PermissionsServiceRequestOptions, ): Promise; } diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index 6e99ff71ab..028517c5e0 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -83,18 +83,22 @@ describe('ServerPermissionClient', () => { const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { discovery, tokenManager: mockServices.tokenManager.mock(), + auth: mockServices.auth(), }); - await client.authorize([ - { - permission: testBasicPermission, - }, - ]); + await client.authorize( + [ + { + permission: testBasicPermission, + }, + ], + { credentials: mockCredentials.none() }, + ); expect(mockAuthorizeHandler).not.toHaveBeenCalled(); }); - it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => { + it('should bypass the permission backend if permissions are enabled and request has valid server credentials', async () => { const client = ServerPermissionClient.fromConfig(config, { discovery, tokenManager: mockServices.tokenManager.mock(), @@ -102,13 +106,13 @@ describe('ServerPermissionClient', () => { }); await client.authorize([{ permission: testBasicPermission }], { - token: mockCredentials.service.token(), + credentials: mockCredentials.service(), }); expect(mockAuthorizeHandler).not.toHaveBeenCalled(); }); - it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => { + it('should call the permission backend if permissions are enabled and request does not have valid server credentials', async () => { const client = ServerPermissionClient.fromConfig(config, { discovery, tokenManager: mockServices.tokenManager.mock(), @@ -116,10 +120,18 @@ describe('ServerPermissionClient', () => { }); await client.authorize([{ permission: testBasicPermission }], { - token: mockCredentials.user.token(), + credentials: mockCredentials.user(), }); expect(mockAuthorizeHandler).toHaveBeenCalled(); + expect( + mockAuthorizeHandler.mock.calls[0][0].headers.get('authorization'), + ).toBe( + mockCredentials.service.header({ + onBehalfOf: mockCredentials.user(), + targetPluginId: 'permission', + }), + ); }); }); @@ -145,201 +157,59 @@ describe('ServerPermissionClient', () => { const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { discovery, tokenManager: mockServices.tokenManager.mock(), - }); - - 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 client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager: mockServices.tokenManager.mock(), + auth: mockServices.auth(), }); await client.authorizeConditional( [{ permission: testResourcePermission }], { - token: mockCredentials.service.token(), + credentials: mockCredentials.none(), }, ); expect(mockAuthorizeHandler).not.toHaveBeenCalled(); }); - it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => { - const auth = mockServices.auth(); + it('should bypass the permission backend if permissions are enabled and request has valid server credentials', async () => { const client = ServerPermissionClient.fromConfig(config, { discovery, tokenManager: mockServices.tokenManager.mock(), - auth, + auth: mockServices.auth(), }); await client.authorizeConditional( [{ permission: testResourcePermission }], { - token: mockCredentials.user.token(), + credentials: mockCredentials.service(), + }, + ); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should call the permission backend if permissions are enabled and request does not have valid server credentials', async () => { + const client = ServerPermissionClient.fromConfig(config, { + discovery, + tokenManager: mockServices.tokenManager.mock(), + auth: mockServices.auth(), + }); + + await client.authorizeConditional( + [{ permission: testResourcePermission }], + { + credentials: mockCredentials.user(), }, ); expect(mockAuthorizeHandler).toHaveBeenCalled(); - }); - }); - - describe('with credentials', () => { - 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)); - }); - - it('should bypass the permission backend if permissions are disabled', async () => { - const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { - discovery, - tokenManager: mockServices.tokenManager.mock(), - auth: mockServices.auth(), - }); - - await client.authorize( - [ - { - permission: testBasicPermission, - }, - ], - { credentials: mockCredentials.none() }, - ); - - expect(mockAuthorizeHandler).not.toHaveBeenCalled(); - }); - - it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => { - const client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager: mockServices.tokenManager.mock(), - auth: mockServices.auth(), - }); - - await client.authorize([{ permission: testBasicPermission }], { - credentials: mockCredentials.service(), - }); - - expect(mockAuthorizeHandler).not.toHaveBeenCalled(); - }); - - it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => { - const client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager: mockServices.tokenManager.mock(), - auth: mockServices.auth(), - }); - - await client.authorize([{ permission: testBasicPermission }], { - credentials: mockCredentials.user(), - }); - - expect(mockAuthorizeHandler).toHaveBeenCalled(); - expect( - mockAuthorizeHandler.mock.calls[0][0].headers.get('authorization'), - ).toBe( - mockCredentials.service.header({ - onBehalfOf: mockCredentials.user(), - targetPluginId: 'permission', - }), - ); - }); - }); - - 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)); - }); - - it('should bypass the permission backend if permissions are disabled', async () => { - const client = ServerPermissionClient.fromConfig(new ConfigReader({}), { - discovery, - tokenManager: mockServices.tokenManager.mock(), - auth: mockServices.auth(), - }); - - await client.authorizeConditional( - [{ permission: testResourcePermission }], - { - credentials: mockCredentials.none(), - }, - ); - - expect(mockAuthorizeHandler).not.toHaveBeenCalled(); - }); - - it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => { - const client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager: mockServices.tokenManager.mock(), - auth: mockServices.auth(), - }); - - await client.authorizeConditional( - [{ permission: testResourcePermission }], - { - credentials: mockCredentials.service(), - }, - ); - - expect(mockAuthorizeHandler).not.toHaveBeenCalled(); - }); - - it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => { - const client = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager: mockServices.tokenManager.mock(), - auth: mockServices.auth(), - }); - - await client.authorizeConditional( - [{ permission: testResourcePermission }], - { - credentials: mockCredentials.user(), - }, - ); - - expect(mockAuthorizeHandler).toHaveBeenCalled(); - expect( - mockAuthorizeHandler.mock.calls[0][0].headers.get('authorization'), - ).toBe( - mockCredentials.service.header({ - onBehalfOf: mockCredentials.user(), - targetPluginId: 'permission', - }), - ); - }); + expect( + mockAuthorizeHandler.mock.calls[0][0].headers.get('authorization'), + ).toBe( + mockCredentials.service.header({ + onBehalfOf: mockCredentials.user(), + targetPluginId: 'permission', + }), + ); }); }); diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index cf54f3b894..e21067bfa1 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -146,14 +146,6 @@ export class ServerPermissionClient implements PermissionsService { return options.credentials; } - if (options?.token) { - try { - return await this.#auth.authenticate(options.token); - } catch { - // ignore - } - } - return undefined; }