diff --git a/.changeset/fifty-insects-yell.md b/.changeset/fifty-insects-yell.md new file mode 100644 index 0000000000..feedd844a5 --- /dev/null +++ b/.changeset/fifty-insects-yell.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +Added `mockServices.userInfo`, which now also automatically is made available in test backends. diff --git a/.changeset/loud-dolls-exist.md b/.changeset/loud-dolls-exist.md new file mode 100644 index 0000000000..5dce8d71f2 --- /dev/null +++ b/.changeset/loud-dolls-exist.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Added the `UserInfoApi` as both an optional input and as an output for `createLegacyAuthAdapters` diff --git a/.changeset/sour-olives-carry.md b/.changeset/sour-olives-carry.md new file mode 100644 index 0000000000..d0c3bdd6d0 --- /dev/null +++ b/.changeset/sour-olives-carry.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Made the `DefaultUserInfoService` claims check stricter diff --git a/.changeset/unlucky-jobs-report.md b/.changeset/unlucky-jobs-report.md new file mode 100644 index 0000000000..80e5ecbea0 --- /dev/null +++ b/.changeset/unlucky-jobs-report.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-permission-backend': patch +--- + +Migrated to use the new auth services introduced in [BEP-0003](https://github.com/backstage/backstage/blob/master/beps/0003-auth-architecture-evolution/README.md). + +The `createRouter` function now has an optional `identity` argument, and instead gained the new `auth`, `httpAuth`, and `userInfo` arguments that should be set to the values of those respective `coreServices`. For users of the new backend system, this happens automatically without code changes. diff --git a/packages/backend-app-api/src/services/implementations/userInfo/userInfoServiceFactory.ts b/packages/backend-app-api/src/services/implementations/userInfo/userInfoServiceFactory.ts index a74b8b7002..7d3a2af7b5 100644 --- a/packages/backend-app-api/src/services/implementations/userInfo/userInfoServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/userInfo/userInfoServiceFactory.ts @@ -43,8 +43,11 @@ export class DefaultUserInfoService implements UserInfoService { if (typeof userEntityRef !== 'string') { throw new Error('User entity ref must be a string'); } - if (!Array.isArray(ownershipEntityRefs)) { - throw new Error('Ownership entity refs must be an array'); + if ( + !Array.isArray(ownershipEntityRefs) || + ownershipEntityRefs.some(ref => typeof ref !== 'string') + ) { + throw new Error('Ownership entity refs must be an array of strings'); } return { userEntityRef, ownershipEntityRefs }; diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 1940a79b78..55cb3b34dc 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -67,6 +67,7 @@ import { ServiceRef } from '@backstage/backend-plugin-api'; import { TokenManagerService as TokenManager } from '@backstage/backend-plugin-api'; import { TransportStreamOptions } from 'winston-transport'; import { UrlReaderService as UrlReader } from '@backstage/backend-plugin-api'; +import { UserInfoService } from '@backstage/backend-plugin-api'; import { V1PodTemplateSpec } from '@kubernetes/client-node'; import * as winston from 'winston'; import { Writable } from 'stream'; @@ -239,30 +240,32 @@ export function createLegacyAuthAdapters< TOptions extends { auth?: AuthService; httpAuth?: HttpAuthService; + userInfo?: UserInfoService; identity?: IdentityService; tokenManager?: TokenManager; discovery: PluginEndpointDiscovery; }, - TAdapters = TOptions extends { + TAdapters = (TOptions extends { auth?: AuthService; } - ? TOptions extends { - httpAuth?: HttpAuthService; + ? { + auth: AuthService; } + : {}) & + (TOptions extends { + httpAuth?: HttpAuthService; + } ? { - auth: AuthService; httpAuth: HttpAuthService; } - : { - auth: AuthService; + : {}) & + (TOptions extends { + userInfo?: UserInfoService; + } + ? { + userInfo: UserInfoService; } - : TOptions extends { - httpAuth?: HttpAuthService; - } - ? { - httpAuth: HttpAuthService; - } - : 'error: at least one of auth and/or httpAuth must be provided', + : {}), >(options: TOptions): TAdapters; // @public diff --git a/packages/backend-common/src/auth/createLegacyAuthAdapters.test.ts b/packages/backend-common/src/auth/createLegacyAuthAdapters.test.ts index 7e1f1be858..db4461781b 100644 --- a/packages/backend-common/src/auth/createLegacyAuthAdapters.test.ts +++ b/packages/backend-common/src/auth/createLegacyAuthAdapters.test.ts @@ -56,7 +56,22 @@ describe('createLegacyAuthAdapters', () => { expect(ret.httpAuth).toBe(httpAuth); }); - it('should adapt both auth and httpAuth if neither are provided', () => { + it('should pass through userInfo if it provided', () => { + const auth = {}; + const userInfo = {}; + const ret = createLegacyAuthAdapters({ + auth: auth as any, + userInfo: userInfo as any, + tokenManager: mockServices.tokenManager(), + discovery: {} as any, + identity: mockServices.identity(), + }); + + expect(ret.auth).toBe(auth); + expect(ret.userInfo).toBe(userInfo); + }); + + it('should adapt all services if none are provided', () => { const ret = createLegacyAuthAdapters({ auth: undefined, httpAuth: undefined, @@ -68,6 +83,7 @@ describe('createLegacyAuthAdapters', () => { expect(ret).toEqual({ auth: expect.any(Object), httpAuth: expect.any(Object), + userInfo: expect.any(Object), }); }); }); diff --git a/packages/backend-common/src/auth/createLegacyAuthAdapters.ts b/packages/backend-common/src/auth/createLegacyAuthAdapters.ts index 98d21ba559..d12dd8b9fc 100644 --- a/packages/backend-common/src/auth/createLegacyAuthAdapters.ts +++ b/packages/backend-common/src/auth/createLegacyAuthAdapters.ts @@ -19,10 +19,12 @@ import { BackstageCredentials, BackstagePrincipalTypes, BackstageServicePrincipal, + BackstageUserInfo, BackstageUserPrincipal, HttpAuthService, IdentityService, TokenManagerService, + UserInfoService, } from '@backstage/backend-plugin-api'; import { ServerTokenManager, TokenManager } from '../tokens'; import { AuthenticationError, NotAllowedError } from '@backstage/errors'; @@ -203,6 +205,35 @@ class HttpAuthCompat implements HttpAuthService { async issueUserCookie(_res: Response): Promise {} } +export class UserInfoCompat implements UserInfoService { + async getUserInfo( + credentials: BackstageCredentials, + ): Promise { + const internalCredentials = toInternalBackstageCredentials(credentials); + if (internalCredentials.principal.type !== 'user') { + throw new Error('Only user credentials are supported'); + } + if (!internalCredentials.token) { + throw new Error('User credentials is unexpectedly missing token'); + } + const { sub: userEntityRef, ent: ownershipEntityRefs = [] } = decodeJwt( + internalCredentials.token, + ); + + if (typeof userEntityRef !== 'string') { + throw new Error('User entity ref must be a string'); + } + if ( + !Array.isArray(ownershipEntityRefs) || + ownershipEntityRefs.some(ref => typeof ref !== 'string') + ) { + throw new Error('Ownership entity refs must be an array of strings'); + } + + return { userEntityRef, ownershipEntityRefs }; + } +} + /** * An adapter that ensures presence of the auth and/or httpAuth services. * @public @@ -211,38 +242,47 @@ export function createLegacyAuthAdapters< TOptions extends { auth?: AuthService; httpAuth?: HttpAuthService; + userInfo?: UserInfoService; identity?: IdentityService; tokenManager?: TokenManager; discovery: PluginEndpointDiscovery; }, - TAdapters = TOptions extends { - auth?: AuthService; - } - ? TOptions extends { httpAuth?: HttpAuthService } - ? { auth: AuthService; httpAuth: HttpAuthService } - : { auth: AuthService } - : TOptions extends { httpAuth?: HttpAuthService } - ? { httpAuth: HttpAuthService } - : 'error: at least one of auth and/or httpAuth must be provided', + TAdapters = (TOptions extends { auth?: AuthService } + ? { auth: AuthService } + : {}) & + (TOptions extends { httpAuth?: HttpAuthService } + ? { httpAuth: HttpAuthService } + : {}) & + (TOptions extends { userInfo?: UserInfoService } + ? { userInfo: UserInfoService } + : {}), >(options: TOptions): TAdapters { - const { auth, httpAuth, discovery } = options; + const { + auth, + httpAuth, + userInfo = new UserInfoCompat(), + discovery, + } = options; if (auth && httpAuth) { return { auth, httpAuth, + userInfo, } as TAdapters; } if (auth) { return { auth, + userInfo, } as TAdapters; } if (httpAuth) { return { httpAuth, + userInfo, } as TAdapters; } @@ -257,5 +297,6 @@ export function createLegacyAuthAdapters< return { auth: authImpl, httpAuth: httpAuthImpl, + userInfo, } as TAdapters; } diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index a9ebb402c4..d0e37a476d 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -12,6 +12,7 @@ import { BackendFeature } from '@backstage/backend-plugin-api'; import { BackstageCredentials } from '@backstage/backend-plugin-api'; import { BackstageNonePrincipal } from '@backstage/backend-plugin-api'; import { BackstageServicePrincipal } from '@backstage/backend-plugin-api'; +import { BackstageUserInfo } from '@backstage/backend-plugin-api'; import { BackstageUserPrincipal } from '@backstage/backend-plugin-api'; import { CacheService } from '@backstage/backend-plugin-api'; import { DatabaseService } from '@backstage/backend-plugin-api'; @@ -37,6 +38,7 @@ import { ServiceFactory } from '@backstage/backend-plugin-api'; import { ServiceRef } from '@backstage/backend-plugin-api'; import { TokenManagerService } from '@backstage/backend-plugin-api'; import { UrlReaderService } from '@backstage/backend-plugin-api'; +import { UserInfoService } from '@backstage/backend-plugin-api'; // @public export function createMockDirectory( @@ -316,6 +318,17 @@ export namespace mockServices { partialImpl?: Partial | undefined, ) => ServiceMock; } + export function userInfo( + customInfo?: Partial, + ): UserInfoService; + // (undocumented) + export namespace userInfo { + const factory: () => ServiceFactory; + const // (undocumented) + mock: ( + partialImpl?: Partial | undefined, + ) => ServiceMock; + } } // @public diff --git a/packages/backend-test-utils/src/next/services/MockUserInfoService.test.ts b/packages/backend-test-utils/src/next/services/MockUserInfoService.test.ts new file mode 100644 index 0000000000..13c8a43213 --- /dev/null +++ b/packages/backend-test-utils/src/next/services/MockUserInfoService.test.ts @@ -0,0 +1,55 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { MockUserInfoService } from './MockUserInfoService'; +import { mockCredentials } from './mockCredentials'; + +describe('MockUserInfoService', () => { + it('works without constructor parameters', async () => { + const service = new MockUserInfoService(); + const user = mockCredentials.user(); + await expect(service.getUserInfo(user)).resolves.toEqual({ + userEntityRef: user.principal.userEntityRef, + ownershipEntityRefs: [user.principal.userEntityRef], + }); + }); + + it('works with custom constructor parameters', async () => { + const service = new MockUserInfoService({ + userEntityRef: 'user:default/not-the-mock-1', + ownershipEntityRefs: ['user:default/not-the-mock-2'], + }); + const user = mockCredentials.user(); + await expect(service.getUserInfo(user)).resolves.toEqual({ + userEntityRef: 'user:default/not-the-mock-1', + ownershipEntityRefs: ['user:default/not-the-mock-2'], + }); + }); + + it('rejects non-users', async () => { + const service = new MockUserInfoService(); + await expect( + service.getUserInfo(mockCredentials.none()), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"User info not available for principal type 'none'"`, + ); + await expect( + service.getUserInfo(mockCredentials.service()), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"User info not available for principal type 'service'"`, + ); + }); +}); diff --git a/packages/backend-test-utils/src/next/services/MockUserInfoService.ts b/packages/backend-test-utils/src/next/services/MockUserInfoService.ts new file mode 100644 index 0000000000..68c2a8acae --- /dev/null +++ b/packages/backend-test-utils/src/next/services/MockUserInfoService.ts @@ -0,0 +1,55 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + BackstageCredentials, + BackstageNonePrincipal, + BackstageServicePrincipal, + BackstageUserInfo, + BackstageUserPrincipal, + UserInfoService, +} from '@backstage/backend-plugin-api'; +import { InputError } from '@backstage/errors'; + +/** @internal */ +export class MockUserInfoService implements UserInfoService { + private readonly customInfo: Partial; + + constructor(customInfo?: Partial) { + this.customInfo = customInfo ?? {}; + } + + async getUserInfo( + credentials: BackstageCredentials, + ): Promise { + const principal = credentials.principal as + | BackstageUserPrincipal + | BackstageServicePrincipal + | BackstageNonePrincipal; + + if (principal.type !== 'user') { + throw new InputError( + `User info not available for principal type '${principal.type}'`, + ); + } + + return { + userEntityRef: principal.userEntityRef, + ownershipEntityRefs: [principal.userEntityRef], + ...this.customInfo, + }; + } +} diff --git a/packages/backend-test-utils/src/next/services/mockServices.ts b/packages/backend-test-utils/src/next/services/mockServices.ts index f72f229434..7300b34105 100644 --- a/packages/backend-test-utils/src/next/services/mockServices.ts +++ b/packages/backend-test-utils/src/next/services/mockServices.ts @@ -27,6 +27,8 @@ import { DiscoveryService, HttpAuthService, BackstageCredentials, + BackstageUserInfo, + UserInfoService, } from '@backstage/backend-plugin-api'; import { cacheServiceFactory, @@ -49,6 +51,7 @@ import { MockRootLoggerService } from './MockRootLoggerService'; import { MockAuthService } from './MockAuthService'; import { MockHttpAuthService } from './MockHttpAuthService'; import { mockCredentials } from './mockCredentials'; +import { MockUserInfoService } from './MockUserInfoService'; /** @internal */ function simpleFactory< @@ -272,6 +275,37 @@ export namespace mockServices { })); } + /** + * Creates a mock implementation of the `UserInfoService`. + * + * By default it extracts the user's entity ref from a user principal and + * returns that as the only ownership entity ref, but this can be overridden + * by passing in a custom set of user info. + */ + export function userInfo( + customInfo?: Partial, + ): UserInfoService { + return new MockUserInfoService(customInfo); + } + export namespace userInfo { + /** + * Creates a mock service factory for the `UserInfoService`. + * + * By default it extracts the user's entity ref from a user principal and + * returns that as the only ownership entity ref. + */ + export const factory = createServiceFactory({ + service: coreServices.userInfo, + deps: {}, + factory() { + return new MockUserInfoService(); + }, + }); + export const mock = simpleMock(coreServices.userInfo, () => ({ + getUserInfo: jest.fn(), + })); + } + // TODO(Rugvip): Not all core services have implementations available here yet. // some may need a bit more refactoring for it to be simpler to // re-implement functioning mock versions here. @@ -284,12 +318,14 @@ export namespace mockServices { withOptions: jest.fn(), })); } + export namespace database { export const factory = databaseServiceFactory; export const mock = simpleMock(coreServices.database, () => ({ getClient: jest.fn(), })); } + export namespace httpRouter { export const factory = httpRouterServiceFactory; export const mock = simpleMock(coreServices.httpRouter, () => ({ @@ -297,12 +333,14 @@ export namespace mockServices { addAuthPolicy: jest.fn(), })); } + export namespace rootHttpRouter { export const factory = rootHttpRouterServiceFactory; export const mock = simpleMock(coreServices.rootHttpRouter, () => ({ use: jest.fn(), })); } + export namespace lifecycle { export const factory = lifecycleServiceFactory; export const mock = simpleMock(coreServices.lifecycle, () => ({ @@ -310,6 +348,7 @@ export namespace mockServices { addStartupHook: jest.fn(), })); } + export namespace logger { export const factory = loggerServiceFactory; export const mock = simpleMock(coreServices.logger, () => ({ @@ -320,6 +359,7 @@ export namespace mockServices { warn: jest.fn(), })); } + export namespace permissions { export const factory = permissionsServiceFactory; export const mock = simpleMock(coreServices.permissions, () => ({ @@ -327,6 +367,7 @@ export namespace mockServices { authorizeConditional: jest.fn(), })); } + export namespace rootLifecycle { export const factory = rootLifecycleServiceFactory; export const mock = simpleMock(coreServices.rootLifecycle, () => ({ @@ -334,6 +375,7 @@ export namespace mockServices { addStartupHook: jest.fn(), })); } + export namespace scheduler { export const factory = schedulerServiceFactory; export const mock = simpleMock(coreServices.scheduler, () => ({ @@ -343,6 +385,7 @@ export namespace mockServices { triggerTask: jest.fn(), })); } + export namespace urlReader { export const factory = urlReaderServiceFactory; export const mock = simpleMock(coreServices.urlReader, () => ({ diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 72a2ed8edb..6b58ebc5ea 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -80,6 +80,7 @@ export const defaultServiceFactories = [ mockServices.rootLogger.factory(), mockServices.scheduler.factory(), mockServices.tokenManager.factory(), + mockServices.userInfo.factory(), mockServices.urlReader.factory(), ]; diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index e9cc4e6272..8b4336ff23 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -3,27 +3,36 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +import { AuthService } from '@backstage/backend-plugin-api'; import { Config } from '@backstage/config'; +import { DiscoveryService } from '@backstage/backend-plugin-api'; import express from 'express'; +import { HttpAuthService } from '@backstage/backend-plugin-api'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { Logger } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { UserInfoService } from '@backstage/backend-plugin-api'; // @public export function createRouter(options: RouterOptions): Promise; // @public export interface RouterOptions { + // (undocumented) + auth?: AuthService; // (undocumented) config: Config; // (undocumented) - discovery: PluginEndpointDiscovery; + discovery: DiscoveryService; // (undocumented) - identity: IdentityApi; + httpAuth?: HttpAuthService; + // (undocumented) + identity?: IdentityApi; // (undocumented) logger: Logger; // (undocumented) policy: PermissionPolicy; + // (undocumented) + userInfo?: UserInfoService; } ``` diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index cc986c4cc8..4131dc29b9 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -62,6 +62,7 @@ "zod": "^3.22.4" }, "devDependencies": { + "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@types/lodash": "^4.14.151", "@types/supertest": "^2.0.8", diff --git a/plugins/permission-backend/src/plugin.ts b/plugins/permission-backend/src/plugin.ts index 9cf0024e47..cc7f31dfdd 100644 --- a/plugins/permission-backend/src/plugin.ts +++ b/plugins/permission-backend/src/plugin.ts @@ -55,9 +55,19 @@ export const permissionPlugin = createBackendPlugin({ config: coreServices.rootConfig, logger: coreServices.logger, discovery: coreServices.discovery, - identity: coreServices.identity, + auth: coreServices.auth, + httpAuth: coreServices.httpAuth, + userInfo: coreServices.userInfo, }, - async init({ http, config, logger, discovery, identity }) { + async init({ + http, + config, + logger, + discovery, + auth, + httpAuth, + userInfo, + }) { const winstonLogger = loggerToWinstonLogger(logger); if (!policies.policy) { throw new Error( @@ -69,9 +79,11 @@ export const permissionPlugin = createBackendPlugin({ await createRouter({ config, discovery, - identity, logger: winstonLogger, policy: policies.policy, + auth, + httpAuth, + userInfo, }), ); }, diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index c7a412547d..a36dadd2b5 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -19,7 +19,7 @@ import { Server } from 'http'; import express, { Router, RequestHandler } from 'express'; import { RestContext, rest } from 'msw'; import { setupServer, SetupServer } from 'msw/node'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { AuthorizeResult, PermissionCondition, @@ -31,10 +31,12 @@ import { } from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { z } from 'zod'; +import { DiscoveryService } from '@backstage/backend-plugin-api'; describe('PermissionIntegrationClient', () => { describe('applyConditions', () => { let server: SetupServer; + const auth = mockServices.auth(); const mockConditions: PermissionCriteria = { not: { @@ -58,7 +60,7 @@ describe('PermissionIntegrationClient', () => { ); const mockBaseUrl = 'http://backstage:9191'; - const discovery: PluginEndpointDiscovery = { + const discovery: DiscoveryService = { async getBaseUrl(pluginId) { return `${mockBaseUrl}/${pluginId}`; }, @@ -70,6 +72,7 @@ describe('PermissionIntegrationClient', () => { const client: PermissionIntegrationClient = new PermissionIntegrationClient( { discovery, + auth, }, ); @@ -91,7 +94,7 @@ describe('PermissionIntegrationClient', () => { }); it('should make a POST request to the correct endpoint', async () => { - await client.applyConditions('plugin-1', [ + await client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -104,7 +107,7 @@ describe('PermissionIntegrationClient', () => { }); it('should include a request body', async () => { - await client.applyConditions('plugin-1', [ + await client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -132,14 +135,18 @@ describe('PermissionIntegrationClient', () => { }); it('should return the response from the fetch request', async () => { - const response = await client.applyConditions('plugin-1', [ - { - id: '123', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: mockConditions, - }, - ]); + const response = await client.applyConditions( + 'plugin-1', + mockCredentials.none(), + [ + { + id: '123', + resourceRef: 'testResource1', + resourceType: 'test-resource', + conditions: mockConditions, + }, + ], + ); expect(response).toEqual( expect.objectContaining([{ id: '123', result: AuthorizeResult.ALLOW }]), @@ -147,7 +154,7 @@ describe('PermissionIntegrationClient', () => { }); it('should not include authorization headers if no token is supplied', async () => { - await client.applyConditions('plugin-1', [ + await client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -161,21 +168,22 @@ describe('PermissionIntegrationClient', () => { }); it('should include correctly-constructed authorization header if token is supplied', async () => { - await client.applyConditions( - 'plugin-1', - [ - { - id: '123', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: mockConditions, - }, - ], - 'Bearer fake-token', - ); + await client.applyConditions('plugin-1', mockCredentials.user(), [ + { + id: '123', + resourceRef: 'testResource1', + resourceType: 'test-resource', + conditions: mockConditions, + }, + ]); const request = mockApplyConditionsHandler.mock.calls[0][0]; - expect(request.headers.get('authorization')).toEqual('Bearer fake-token'); + expect(request.headers.get('authorization')).toEqual( + mockCredentials.service.header({ + onBehalfOf: mockCredentials.user(), + targetPluginId: 'plugin-1', + }), + ); }); it('should forward response errors', async () => { @@ -186,7 +194,7 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions('plugin-1', [ + client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -194,7 +202,7 @@ describe('PermissionIntegrationClient', () => { conditions: mockConditions, }, ]), - ).rejects.toThrow(/401/i); + ).rejects.toThrow(/401/); }); it('should reject invalid responses', async () => { @@ -207,7 +215,7 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions('plugin-1', [ + client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -234,7 +242,7 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions('plugin-1', [ + client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -268,6 +276,7 @@ describe('PermissionIntegrationClient', () => { let server: Server; let client: PermissionIntegrationClient; let routerSpy: RequestHandler; + const auth = mockServices.auth(); beforeAll(async () => { const router = Router(); @@ -319,7 +328,7 @@ describe('PermissionIntegrationClient', () => { server = app.listen(resolve); }); - const discovery: PluginEndpointDiscovery = { + const discovery: DiscoveryService = { async getBaseUrl(pluginId: string) { const listenPort = (server.address()! as AddressInfo).port; @@ -332,6 +341,7 @@ describe('PermissionIntegrationClient', () => { client = new PermissionIntegrationClient({ discovery, + auth, }); }); @@ -348,7 +358,7 @@ describe('PermissionIntegrationClient', () => { it('works for simple conditions', async () => { await expect( - client.applyConditions('plugin-1', [ + client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', @@ -367,7 +377,7 @@ describe('PermissionIntegrationClient', () => { it('works for complex criteria', async () => { await expect( - client.applyConditions('plugin-1', [ + client.applyConditions('plugin-1', mockCredentials.none(), [ { id: '123', resourceRef: 'testResource1', diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts index 2c31d371c3..7567dc8fbb 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts @@ -16,7 +16,6 @@ import fetch from 'node-fetch'; import { z } from 'zod'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { AuthorizeResult, ConditionalPolicyDecision, @@ -25,6 +24,11 @@ import { ApplyConditionsRequestEntry, ApplyConditionsResponseEntry, } from '@backstage/plugin-permission-node'; +import { + AuthService, + BackstageCredentials, + DiscoveryService, +} from '@backstage/backend-plugin-api'; const responseSchema = z.object({ items: z.array( @@ -42,20 +46,30 @@ export type ResourcePolicyDecision = ConditionalPolicyDecision & { }; export class PermissionIntegrationClient { - private readonly discovery: PluginEndpointDiscovery; + private readonly discovery: DiscoveryService; + private readonly auth: AuthService; - constructor(options: { discovery: PluginEndpointDiscovery }) { + constructor(options: { discovery: DiscoveryService; auth: AuthService }) { this.discovery = options.discovery; + this.auth = options.auth; } async applyConditions( pluginId: string, + credentials: BackstageCredentials, decisions: readonly ApplyConditionsRequestEntry[], - authHeader?: string, ): Promise { - const endpoint = `${await this.discovery.getBaseUrl( - pluginId, - )}/.well-known/backstage/permissions/apply-conditions`; + const baseUrl = await this.discovery.getBaseUrl(pluginId); + const endpoint = `${baseUrl}/.well-known/backstage/permissions/apply-conditions`; + + const token = this.auth.isPrincipal(credentials, 'none') + ? undefined + : await this.auth + .getPluginRequestToken({ + onBehalfOf: credentials, + targetPluginId: pluginId, + }) + .then(t => t.token); const response = await fetch(endpoint, { method: 'POST', @@ -70,7 +84,7 @@ export class PermissionIntegrationClient { ), }), headers: { - ...(authHeader ? { authorization: authHeader } : {}), + ...(token ? { authorization: `Bearer ${token}` } : {}), 'content-type': 'application/json', }, }); diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 7278835c5c..0da9ecd748 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -26,12 +26,15 @@ import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { createRouter } from './router'; import { ConfigReader } from '@backstage/config'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; const mockApplyConditions: jest.MockedFunction< InstanceType['applyConditions'] > = jest.fn( async ( _pluginId: string, + _credentials: BackstageCredentials, decisions: readonly ApplyConditionsRequestEntry[], ) => decisions.map(decision => ({ @@ -65,28 +68,12 @@ describe('createRouter', () => { const router = await createRouter({ config: new ConfigReader({ permission: { enabled: true } }), logger: getVoidLogger(), - discovery: { - getBaseUrl: jest.fn(), - getExternalBaseUrl: jest.fn(), - }, - identity: { - getIdentity: jest.fn(({ request: req }) => { - const token = req.headers.authorization?.replace(/^Bearer[ ]+/, ''); - - if (!token) { - return Promise.resolve(undefined); - } - - return Promise.resolve({ - identity: { - type: 'user', - userEntityRef: 'test-user', - ownershipEntityRefs: ['blah'], - }, - token, - }); - }), - }, + discovery: mockServices.discovery(), + auth: mockServices.auth(), + httpAuth: mockServices.httpAuth({ + defaultCredentials: mockCredentials.none(), + }), + userInfo: mockServices.userInfo(), policy, }); @@ -163,10 +150,9 @@ describe('createRouter', () => { }); it('resolves identity from the Authorization header', async () => { - const token = 'test-token'; const response = await request(app) .post('/authorize') - .auth(token, { type: 'bearer' }) + .auth(mockCredentials.user.token(), { type: 'bearer' }) .send({ items: [ { @@ -190,11 +176,16 @@ describe('createRouter', () => { }, }, { - token: 'test-token', + token: mockCredentials.service.token({ + onBehalfOf: mockCredentials.user(), + targetPluginId: 'catalog', + }), identity: { type: 'user', - userEntityRef: 'test-user', - ownershipEntityRefs: ['blah'], + userEntityRef: mockCredentials.user().principal.userEntityRef, + ownershipEntityRefs: [ + mockCredentials.user().principal.userEntityRef, + ], }, }, ); @@ -271,7 +262,7 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .auth('test-token', { type: 'bearer' }) + .auth(mockCredentials.user.token(), { type: 'bearer' }) .send({ items: [ { @@ -319,6 +310,7 @@ describe('createRouter', () => { expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', + mockCredentials.user(), [ expect.objectContaining({ id: '123', @@ -333,11 +325,11 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params: ['no'] }, }), ], - 'Bearer test-token', ); expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-2', + mockCredentials.user(), [ expect.objectContaining({ id: '234', @@ -352,7 +344,6 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params: ['no'] }, }), ], - 'Bearer test-token', ); expect(response.status).toEqual(200); @@ -401,7 +392,7 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .auth('test-token', { type: 'bearer' }) + .auth(mockCredentials.user.token(), { type: 'bearer' }) .send({ items: [ { @@ -467,6 +458,7 @@ describe('createRouter', () => { expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', + mockCredentials.user(), [ expect.objectContaining({ id: '123', @@ -481,11 +473,11 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params: ['yes'] }, }), ], - 'Bearer test-token', ); expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-2', + mockCredentials.user(), [ expect.objectContaining({ id: '234', @@ -500,7 +492,6 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params: ['yes'] }, }), ], - 'Bearer test-token', ); expect(response.status).toEqual(200); @@ -542,7 +533,7 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .auth('test-token', { type: 'bearer' }) + .auth(mockCredentials.user.token(), { type: 'bearer' }) .send({ items: [ { @@ -589,6 +580,7 @@ describe('createRouter', () => { expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', + mockCredentials.user(), [ expect.objectContaining({ id: '123', @@ -597,11 +589,11 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params: ['yes'] }, }), ], - 'Bearer test-token', ); expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-2', + mockCredentials.user(), [ expect.objectContaining({ id: '234', @@ -610,7 +602,6 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params: ['yes'] }, }), ], - 'Bearer test-token', ); expect(response.status).toEqual(200); @@ -656,7 +647,7 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .auth('test-token', { type: 'bearer' }) + .auth(mockCredentials.user.token(), { type: 'bearer' }) .send({ items: [ { @@ -684,6 +675,7 @@ describe('createRouter', () => { expect(mockApplyConditions).toHaveBeenCalledWith( 'test-plugin', + mockCredentials.user(), [ expect.objectContaining({ id: '123', @@ -698,7 +690,6 @@ describe('createRouter', () => { conditions: { rule: 'test-rule', params }, }), ], - 'Bearer test-token', ); expect(response.status).toEqual(200); diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index b7e77fdba9..cd7c71fd87 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -19,8 +19,8 @@ import express, { Request, Response } from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { + createLegacyAuthAdapters, errorHandler, - PluginEndpointDiscovery, } from '@backstage/backend-common'; import { InputError } from '@backstage/errors'; import { @@ -46,6 +46,15 @@ import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { memoize } from 'lodash'; import DataLoader from 'dataloader'; import { Config } from '@backstage/config'; +import { + AuthService, + BackstageCredentials, + BackstageNonePrincipal, + BackstageUserPrincipal, + DiscoveryService, + HttpAuthService, + UserInfoService, +} from '@backstage/backend-plugin-api'; const attributesSchema: z.ZodSchema = z.object({ action: z @@ -93,28 +102,51 @@ const evaluatePermissionRequestBatchSchema: z.ZodSchema[], - user: BackstageIdentityResponse | undefined, policy: PermissionPolicy, permissionIntegrationClient: PermissionIntegrationClient, - authHeader?: string, + credentials: BackstageCredentials< + BackstageNonePrincipal | BackstageUserPrincipal + >, + auth: AuthService, + userInfo: UserInfoService, ): Promise[]> => { const applyConditionsLoaderFor = memoize((pluginId: string) => { return new DataLoader< ApplyConditionsRequestEntry, ApplyConditionsResponseEntry >(batch => - permissionIntegrationClient.applyConditions(pluginId, batch, authHeader), + permissionIntegrationClient.applyConditions(pluginId, credentials, batch), ); }); + let user: BackstageIdentityResponse | undefined; + if (auth.isPrincipal(credentials, 'user')) { + const { ownershipEntityRefs } = await userInfo.getUserInfo(credentials); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: credentials, + targetPluginId: 'catalog', // TODO: unknown at this point + }); + user = { + identity: { + type: 'user', + userEntityRef: credentials.principal.userEntityRef, + ownershipEntityRefs, + }, + token, + }; + } + return Promise.all( requests.map(({ id, resourceRef, ...request }) => policy.handle(request, user).then(decision => { @@ -163,7 +195,8 @@ const handleRequest = async ( export async function createRouter( options: RouterOptions, ): Promise { - const { policy, discovery, identity, config, logger } = options; + const { policy, discovery, config, logger } = options; + const { auth, httpAuth, userInfo } = createLegacyAuthAdapters(options); if (!config.getOptionalBoolean('permission.enabled')) { logger.warn( @@ -173,6 +206,7 @@ export async function createRouter( const permissionIntegrationClient = new PermissionIntegrationClient({ discovery, + auth, }); const router = Router(); @@ -188,7 +222,9 @@ export async function createRouter( req: Request, res: Response, ) => { - const user = await identity.getIdentity({ request: req }); + const credentials = await httpAuth.credentials(req, { + allow: ['user', 'none'], + }); const parseResult = evaluatePermissionRequestBatchSchema.safeParse( req.body, @@ -203,10 +239,11 @@ export async function createRouter( res.json({ items: await handleRequest( body.items, - user, policy, permissionIntegrationClient, - req.header('authorization'), + credentials, + auth, + userInfo, ), }); }, diff --git a/yarn.lock b/yarn.lock index 5aa82df2e2..1dc69f711f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7917,6 +7917,7 @@ __metadata: dependencies: "@backstage/backend-common": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" + "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^"