diff --git a/.changeset/friendly-keys-fold.md b/.changeset/friendly-keys-fold.md new file mode 100644 index 0000000000..218abe4efe --- /dev/null +++ b/.changeset/friendly-keys-fold.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-plugin-api': patch +'@backstage/backend-app-api': patch +--- + +Added an optional `accessRestrictions` to external access service tokens and service principals in general, such that you can limit their access to certain plugins or permissions. diff --git a/.changeset/great-cougars-guess.md b/.changeset/great-cougars-guess.md new file mode 100644 index 0000000000..af1de3e8ce --- /dev/null +++ b/.changeset/great-cougars-guess.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +Made it possible to give access restrictions to `mockCredentials.service` diff --git a/.changeset/neat-rivers-share.md b/.changeset/neat-rivers-share.md new file mode 100644 index 0000000000..42649de977 --- /dev/null +++ b/.changeset/neat-rivers-share.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': patch +--- + +Ensure that service token access restrictions, when present, are taken into account diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts index 5517af4f98..5b72fef54c 100644 --- a/packages/backend-app-api/config.d.ts +++ b/packages/backend-app-api/config.d.ts @@ -88,6 +88,46 @@ export interface Config { */ subject: string; }; + /** + * Restricts what types of access that are permitted for this access + * method. If no access restrictions are given, it'll have unlimited + * access. This access restriction applies for the framework level; + * individual plugins may have their own access control mechanisms + * on top of this. + */ + accessRestrictions?: Array<{ + /** + * Permit access to make requests to this plugin. + * + * Can be further refined by setting additional fields below. + */ + plugin: string; + /** + * If given, this method is limited to only performing actions + * with these named permissions in this plugin. + * + * Note that this only applies where permissions checks are + * enabled in the first place. Endpoints that are not protected by + * the permissions system at all, are not affected by this + * setting. + */ + permission?: string | Array; + /** + * If given, this method is limited to only performing actions + * whose permissions have these attributes. + * + * Note that this only applies where permissions checks are + * enabled in the first place. Endpoints that are not protected by + * the permissions system at all, are not affected by this + * setting. + */ + permissionAttribute?: { + /** + * One of more of 'create', 'read', 'update', or 'delete'. + */ + action?: string | Array; + }; + }>; } | { /** @@ -130,6 +170,46 @@ export interface Config { */ subject: string; }; + /** + * Restricts what types of access that are permitted for this access + * method. If no access restrictions are given, it'll have unlimited + * access. This access restriction applies for the framework level; + * individual plugins may have their own access control mechanisms + * on top of this. + */ + accessRestrictions?: Array<{ + /** + * Permit access to make requests to this plugin. + * + * Can be further refined by setting additional fields below. + */ + plugin: string; + /** + * If given, this method is limited to only performing actions + * with these named permissions in this plugin. + * + * Note that this only applies where permissions checks are + * enabled in the first place. Endpoints that are not protected by + * the permissions system at all, are not affected by this + * setting. + */ + permission?: string | Array; + /** + * If given, this method is limited to only performing actions + * whose permissions have these attributes. + * + * Note that this only applies where permissions checks are + * enabled in the first place. Endpoints that are not protected by + * the permissions system at all, are not affected by this + * setting. + */ + permissionAttribute?: { + /** + * One of more of 'create', 'read', 'update', or 'delete'. + */ + action?: string | Array; + }; + }>; } | { /** diff --git a/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts b/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts index dc76a8761b..b4feb46cce 100644 --- a/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts +++ b/packages/backend-app-api/src/services/implementations/auth/DefaultAuthService.ts @@ -82,7 +82,11 @@ export class DefaultAuthService implements AuthService { const externalResult = await this.externalTokenHandler.verifyToken(token); if (externalResult) { - return createCredentialsWithServicePrincipal(externalResult.subject); + return createCredentialsWithServicePrincipal( + externalResult.subject, + undefined, + externalResult.accessRestrictions, + ); } throw new AuthenticationError('Illegal token'); diff --git a/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.test.ts b/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.test.ts index 96d4ce4dc9..71fbe64963 100644 --- a/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.test.ts @@ -42,7 +42,26 @@ const mockDeps = [ data: { backend: { baseUrl: 'http://localhost', - auth: { keys: [{ secret: 'abc' }] }, + auth: { + keys: [{ secret: 'abc' }], + externalAccess: [ + { + type: 'static', + options: { + token: 'limited-static-token', + subject: 'limited-static-subject', + }, + accessRestrictions: [{ plugin: 'catalog', permission: 'do.it' }], + }, + { + type: 'static', + options: { + token: 'unlimited-static-token', + subject: 'unlimited-static-subject', + }, + }, + ], + }, }, }, }), @@ -385,4 +404,43 @@ describe('authServiceFactory', () => { "Unable to call 'kubernetes' plugin on behalf of user, because the target plugin does not support on-behalf-of tokens or the plugin doesn't exist", ); }); + + it('should eagerly reject access to external access tokens based on plugin id', async () => { + const tester = ServiceFactoryTester.from(authServiceFactory, { + dependencies: mockDeps, + }); + + const catalogAuth = await tester.get('catalog'); + + await expect( + catalogAuth.authenticate('limited-static-token'), + ).resolves.toMatchObject({ + principal: { + subject: 'limited-static-subject', + accessRestrictions: { permissionNames: ['do.it'] }, + }, + }); + + await expect( + catalogAuth.authenticate('unlimited-static-token'), + ).resolves.toMatchObject({ + principal: { + subject: 'unlimited-static-subject', + }, + }); + + const scaffolderAuth = await tester.get('scaffolder'); + + await expect( + scaffolderAuth.authenticate('limited-static-token'), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"This token's access is restricted to plugin(s) 'catalog'"`, + ); + + await expect( + scaffolderAuth.authenticate('unlimited-static-token'), + ).resolves.toMatchObject({ + principal: { subject: 'unlimited-static-subject' }, + }); + }); }); diff --git a/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts b/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts index 7bbd012c01..ad4fb17d88 100644 --- a/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/auth/authServiceFactory.ts @@ -39,19 +39,7 @@ export const authServiceFactory = createServiceFactory({ // new auth services in the new backend system. tokenManager: coreServices.tokenManager, }, - async createRootContext({ config, logger }) { - const externalTokens = ExternalTokenHandler.create({ - config, - logger, - }); - return { - externalTokens, - }; - }, - async factory( - { config, discovery, plugin, tokenManager, logger, database }, - { externalTokens }, - ) { + async factory({ config, discovery, plugin, tokenManager, logger, database }) { const disableDefaultAuthPolicy = Boolean( config.getOptionalBoolean( 'backend.auth.dangerouslyDisableDefaultAuthPolicy', @@ -73,6 +61,11 @@ export const authServiceFactory = createServiceFactory({ publicKeyStore, discovery, }); + const externalTokens = ExternalTokenHandler.create({ + ownPluginId: plugin.getId(), + config, + logger, + }); return new DefaultAuthService( userTokens, diff --git a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.test.ts new file mode 100644 index 0000000000..1e82f76134 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.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 { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api'; +import { ExternalTokenHandler } from './ExternalTokenHandler'; +import { TokenHandler } from './types'; + +describe('ExternalTokenHandler', () => { + it('skips over inner handlers that do not match, and applies plugin restrictions', async () => { + const handler1: TokenHandler = { + add: jest.fn(), + verifyToken: jest.fn().mockResolvedValue(undefined), + }; + + const handler2: TokenHandler = { + add: jest.fn(), + verifyToken: jest.fn().mockResolvedValue({ + subject: 'sub', + allAccessRestrictions: new Map( + Object.entries({ + plugin1: { + permissionNames: ['do.it'], + } satisfies BackstagePrincipalAccessRestrictions, + }), + ), + }), + }; + + const plugin1 = new ExternalTokenHandler('plugin1', [handler1, handler2]); + const plugin2 = new ExternalTokenHandler('plugin2', [handler1, handler2]); + + await expect(plugin1.verifyToken('token')).resolves.toEqual({ + subject: 'sub', + accessRestrictions: { permissionNames: ['do.it'] }, + }); + await expect( + plugin2.verifyToken('token'), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"This token's access is restricted to plugin(s) 'plugin1'"`, + ); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts index 79ad8dd3c4..72eeae6fcf 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/ExternalTokenHandler.ts @@ -15,16 +15,19 @@ */ import { + BackstagePrincipalAccessRestrictions, LoggerService, RootConfigService, } from '@backstage/backend-plugin-api'; +import { NotAllowedError } from '@backstage/errors'; import { LegacyTokenHandler } from './legacy'; import { StaticTokenHandler } from './static'; -import { TokenHandler } from './types'; import { JWKSHandler } from './jwks'; +import { TokenHandler } from './types'; const NEW_CONFIG_KEY = 'backend.auth.externalAccess'; const OLD_CONFIG_KEY = 'backend.auth.keys'; +let loggedDeprecationWarning = false; /** * Handles all types of external caller token types (i.e. not Backstage user @@ -34,10 +37,11 @@ const OLD_CONFIG_KEY = 'backend.auth.keys'; */ export class ExternalTokenHandler { static create(options: { + ownPluginId: string; config: RootConfigService; logger: LoggerService; }): ExternalTokenHandler { - const { config, logger } = options; + const { ownPluginId, config, logger } = options; const staticHandler = new StaticTokenHandler(); const legacyHandler = new LegacyTokenHandler(); @@ -61,12 +65,13 @@ export class ExternalTokenHandler { `Unknown type '${type}' in ${NEW_CONFIG_KEY}, expected one of ${valid}`, ); } - handler.add(handlerConfig.getConfig('options')); + handler.add(handlerConfig); } // Load the old keys too const legacyConfigs = config.getOptionalConfigArray(OLD_CONFIG_KEY) ?? []; - if (legacyConfigs.length) { + if (legacyConfigs.length && !loggedDeprecationWarning) { + loggedDeprecationWarning = true; logger.warn( `DEPRECATION WARNING: The ${OLD_CONFIG_KEY} config has been replaced by ${NEW_CONFIG_KEY}, see https://backstage.io/docs/auth/service-to-service-auth`, ); @@ -75,18 +80,48 @@ export class ExternalTokenHandler { legacyHandler.addOld(handlerConfig); } - return new ExternalTokenHandler(Object.values(handlers)); + return new ExternalTokenHandler(ownPluginId, Object.values(handlers)); } - constructor(private readonly handlers: TokenHandler[]) {} + constructor( + private readonly ownPluginId: string, + private readonly handlers: TokenHandler[], + ) {} - async verifyToken(token: string): Promise<{ subject: string } | undefined> { + async verifyToken(token: string): Promise< + | { + subject: string; + accessRestrictions?: BackstagePrincipalAccessRestrictions; + } + | undefined + > { for (const handler of this.handlers) { const result = await handler.verifyToken(token); if (result) { - return result; + const { allAccessRestrictions, ...rest } = result; + if (allAccessRestrictions) { + const accessRestrictions = allAccessRestrictions.get( + this.ownPluginId, + ); + if (!accessRestrictions) { + const valid = [...allAccessRestrictions.keys()] + .map(k => `'${k}'`) + .join(', '); + throw new NotAllowedError( + `This token's access is restricted to plugin(s) ${valid}`, + ); + } + + return { + ...rest, + accessRestrictions, + }; + } + + return rest; } } + return undefined; } } diff --git a/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts new file mode 100644 index 0000000000..ee0dc3cb87 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/auth/external/helpers.test.ts @@ -0,0 +1,181 @@ +/* + * 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 { ConfigReader } from '@backstage/config'; +import { readAccessRestrictionsFromConfig } from './helpers'; +import { JsonObject } from '@backstage/types'; + +describe('readAccessRestrictionsFromConfig', () => { + function r(config: JsonObject) { + return readAccessRestrictionsFromConfig(new ConfigReader(config)); + } + + it('handles empty / missing restrictions', () => { + expect(r({})).toBeUndefined(); + expect(r({ accessRestrictions: [] })).toBeUndefined(); + }); + + it('handles type errors', () => { + expect(() => + r({ accessRestrictions: 7 }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions' in 'mock-config', got number, wanted object-array"`, + ); + expect(() => + r({ accessRestrictions: ['hello'] }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0]' in 'mock-config', got string, wanted object-array"`, + ); + expect(() => + r({ accessRestrictions: [{ unknown: {} }] }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid key 'unknown' in 'accessRestrictions' config, expected one of 'plugin', 'permission', 'permissionAttribute'"`, + ); + expect(() => + r({ accessRestrictions: [{ plugin: 7 }] }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0].plugin' in 'mock-config', got number, wanted string"`, + ); + expect(() => + r({ accessRestrictions: [{ plugin: 'valid', permission: 7 }] }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0].permission' in 'mock-config', got number, wanted string"`, + ); + expect(() => + r({ accessRestrictions: [{ plugin: 'valid', permission: [7] }] }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0].permission[0]' in 'mock-config', got number, wanted string-array"`, + ); + expect(() => + r({ accessRestrictions: [{ plugin: 'valid', permissionAttribute: 7 }] }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0].permissionAttribute' in 'mock-config', got number, wanted object"`, + ); + expect(() => + r({ + accessRestrictions: [ + { plugin: 'valid', permissionAttribute: { a: [] } }, + ], + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid key 'a' in 'permissionAttribute' config, expected 'action'"`, + ); + expect(() => + r({ + accessRestrictions: [ + { plugin: 'valid', permissionAttribute: { action: 7 } }, + ], + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0].permissionAttribute.action' in 'mock-config', got number, wanted string"`, + ); + expect(() => + r({ + accessRestrictions: [ + { plugin: 'valid', permissionAttribute: { action: 'wrong' } }, + ], + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid value 'wrong' at 'action' in 'permissionAttributes' config, valid values are 'create', 'read', 'update', 'delete'"`, + ); + }); + + it('parses valid access restrictions', () => { + expect( + r({ + accessRestrictions: [ + { + plugin: 'a', + }, + ], + }), + ).toEqual( + new Map( + Object.entries({ + a: {}, + }), + ), + ); + + expect( + r({ + accessRestrictions: [ + { + plugin: 'a', + permission: 'a, b a', + }, + ], + }), + ).toEqual( + new Map( + Object.entries({ + a: { permissionNames: ['a', 'b'] }, + }), + ), + ); + + expect( + r({ + accessRestrictions: [ + { + plugin: 'a', + permission: ['a', 'b', 'a'], + }, + ], + }), + ).toEqual( + new Map( + Object.entries({ + a: { permissionNames: ['a', 'b'] }, + }), + ), + ); + + expect( + r({ + accessRestrictions: [ + { + plugin: 'a', + permissionAttribute: { action: 'read, update read' }, + }, + ], + }), + ).toEqual( + new Map( + Object.entries({ + a: { permissionAttributes: { action: ['read', 'update'] } }, + }), + ), + ); + + expect( + r({ + accessRestrictions: [ + { + plugin: 'a', + permissionAttribute: { action: ['read', 'update', 'read'] }, + }, + ], + }), + ).toEqual( + new Map( + Object.entries({ + a: { permissionAttributes: { action: ['read', 'update'] } }, + }), + ), + ); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts b/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts new file mode 100644 index 0000000000..5d199a5067 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/auth/external/helpers.ts @@ -0,0 +1,144 @@ +/* + * 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 { Config } from '@backstage/config'; +import { AccessRestriptionsMap } from './types'; + +/** + * Parses and returns the `accessRestrictions` configuration from an + * `externalAccess` entry, or undefined if there wasn't one. + * + * @internal + */ +export function readAccessRestrictionsFromConfig( + externalAccessEntryConfig: Config, +): AccessRestriptionsMap | undefined { + const configs = + externalAccessEntryConfig.getOptionalConfigArray('accessRestrictions') ?? + []; + + const result: AccessRestriptionsMap = new Map(); + for (const config of configs) { + const validKeys = ['plugin', 'permission', 'permissionAttribute']; + for (const key of config.keys()) { + if (!validKeys.includes(key)) { + const valid = validKeys.map(k => `'${k}'`).join(', '); + throw new Error( + `Invalid key '${key}' in 'accessRestrictions' config, expected one of ${valid}`, + ); + } + } + + const pluginId = config.getString('plugin'); + const permissionNames = readPermissionNames(config); + const permissionAttributes = readPermissionAttributes(config); + + if (result.has(pluginId)) { + throw new Error( + `Attempted to declare 'accessRestrictions' twice for plugin '${pluginId}', which is not permitted`, + ); + } + + result.set(pluginId, { + ...(permissionNames ? { permissionNames } : {}), + ...(permissionAttributes ? { permissionAttributes } : {}), + }); + } + + return result.size ? result : undefined; +} + +/** + * Reads a config value as a string or an array of strings, and deduplicates and + * splits by comma/space into a string array. Can also validate against a known + * set of values. Returns undefined if the key didn't exist or if the array + * would have ended up being empty. + */ +function stringOrStringArray( + root: Config, + key: string, + validValues?: readonly T[], +): T[] | undefined { + if (!root.has(key)) { + return undefined; + } + + const rawValues = Array.isArray(root.get(key)) + ? root.getStringArray(key) + : [root.getString(key)]; + + const values = [ + ...new Set( + rawValues + .map(v => v.split(/[ ,]/)) + .flat() + .filter(Boolean), + ), + ]; + + if (!values.length) { + return undefined; + } + + if (validValues?.length) { + for (const value of values) { + if (!validValues.includes(value as T)) { + const valid = validValues.map(k => `'${k}'`).join(', '); + throw new Error( + `Invalid value '${value}' at '${key}' in 'permissionAttributes' config, valid values are ${valid}`, + ); + } + } + } + + return values as T[]; +} + +function readPermissionNames(externalAccessEntryConfig: Config) { + return stringOrStringArray(externalAccessEntryConfig, 'permission'); +} + +function readPermissionAttributes(externalAccessEntryConfig: Config) { + const config = externalAccessEntryConfig.getOptionalConfig( + 'permissionAttribute', + ); + if (!config) { + return undefined; + } + + const validKeys = ['action']; + for (const key of config.keys()) { + if (!validKeys.includes(key)) { + const valid = validKeys.map(k => `'${k}'`).join(', '); + throw new Error( + `Invalid key '${key}' in 'permissionAttribute' config, expected ${valid}`, + ); + } + } + + const action = stringOrStringArray(config, 'action', [ + 'create', + 'read', + 'update', + 'delete', + ]); + + const result = { + ...(action ? { action } : {}), + }; + + return Object.keys(result).length ? result : undefined; +} diff --git a/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts index 8b2469f6b7..c674e46ec9 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/legacy.test.ts @@ -25,17 +25,35 @@ describe('LegacyTokenHandler', () => { const key1 = randomBytes(24); const key2 = randomBytes(24); const key3 = randomBytes(24); + const accessRestrictions1 = new Map( + Object.entries({ + scaffolder: {}, + }), + ); + const accessRestrictions2 = new Map( + Object.entries({ + catalog: { permissionNames: ['catalog.entity.read'] }, + }), + ); tokenHandler.add( new ConfigReader({ - secret: key1.toString('base64'), - subject: 'key1', + options: { + secret: key1.toString('base64'), + subject: 'key1', + }, + accessRestrictions: [{ plugin: 'scaffolder' }], }), ); tokenHandler.add( new ConfigReader({ - secret: key2.toString('base64'), - subject: 'key2', + options: { + secret: key2.toString('base64'), + subject: 'key2', + }, + accessRestrictions: [ + { plugin: 'catalog', permission: 'catalog.entity.read' }, + ], }), ); tokenHandler.addOld( @@ -54,6 +72,7 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token1)).resolves.toEqual({ subject: 'key1', + allAccessRestrictions: accessRestrictions1, }); const token2 = await new SignJWT({ @@ -65,6 +84,7 @@ describe('LegacyTokenHandler', () => { await expect(tokenHandler.verifyToken(token2)).resolves.toEqual({ subject: 'key2', + allAccessRestrictions: accessRestrictions2, }); const token3 = await new SignJWT({ @@ -147,39 +167,93 @@ describe('LegacyTokenHandler', () => { // new style add, bad secrets expect(() => - handler.add(new ConfigReader({ _missingsecret: true, subject: 'ok' })), - ).toThrow(/secret/); + handler.add( + new ConfigReader({ options: { _missingsecret: true, subject: 'ok' } }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Missing required config value at 'options.secret' in 'mock-config'"`, + ); expect(() => - handler.add(new ConfigReader({ secret: '', subject: 'ok' })), - ).toThrow(/secret/); + handler.add(new ConfigReader({ options: { secret: '', subject: 'ok' } })), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'options.secret' in 'mock-config', got empty-string, wanted string"`, + ); expect(() => - handler.add(new ConfigReader({ secret: 'has spaces', subject: 'ok' })), - ).toThrow(/secret/); + handler.add( + new ConfigReader({ options: { secret: 'has spaces', subject: 'ok' } }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal secret, must be a valid base64 string"`, + ); expect(() => - handler.add(new ConfigReader({ secret: 'hasnewline\n', subject: 'ok' })), - ).toThrow(/secret/); + handler.add( + new ConfigReader({ + options: { secret: 'hasnewline\n', subject: 'ok' }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal secret, must be a valid base64 string"`, + ); expect(() => - handler.add(new ConfigReader({ secret: 3, subject: 'ok' })), - ).toThrow(/secret/); + handler.add(new ConfigReader({ options: { secret: 3, subject: 'ok' } })), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'options.secret' in 'mock-config', got number, wanted string"`, + ); // new style add, bad subjects expect(() => - handler.add(new ConfigReader({ secret: 'b2s=', _missingsubject: true })), - ).toThrow(/subject/); - expect(() => - handler.add(new ConfigReader({ secret: 'b2s=', subject: '' })), - ).toThrow(/subject/); - expect(() => - handler.add(new ConfigReader({ secret: 'b2s=', subject: 'has spaces' })), - ).toThrow(/subject/); + handler.add( + new ConfigReader({ + options: { secret: 'b2s=', _missingsubject: true }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Missing required config value at 'options.subject' in 'mock-config'"`, + ); expect(() => handler.add( - new ConfigReader({ secret: 'b2s=', subject: 'hasnewline\n' }), + new ConfigReader({ options: { secret: 'b2s=', subject: '' } }), ), - ).toThrow(/subject/); + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'options.subject' in 'mock-config', got empty-string, wanted string"`, + ); expect(() => - handler.add(new ConfigReader({ secret: 'b2s=', subject: 3 })), - ).toThrow(/subject/); + handler.add( + new ConfigReader({ + options: { secret: 'b2s=', subject: 'has spaces' }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal subject, must be a set of non-space characters"`, + ); + expect(() => + handler.add( + new ConfigReader({ + options: { secret: 'b2s=', subject: 'hasnewline\n' }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal subject, must be a set of non-space characters"`, + ); + expect(() => + handler.add( + new ConfigReader({ options: { secret: 'b2s=', subject: 3 } }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'options.subject' in 'mock-config', got number, wanted string"`, + ); + + // new style add, bad access restrictions + expect(() => + handler.add( + new ConfigReader({ + options: { secret: 'b2s=', subject: 'subject' }, + accessRestrictions: [{ plugin: ['a'] }], + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'accessRestrictions[0].plugin' in 'mock-config', got array, wanted string"`, + ); // old style add expect(() => @@ -187,18 +261,28 @@ describe('LegacyTokenHandler', () => { ).not.toThrow(); expect(() => handler.addOld(new ConfigReader({ _missingsecret: true })), - ).toThrow(/secret/); - expect(() => handler.addOld(new ConfigReader({ secret: '' }))).toThrow( - /secret/, + ).toThrowErrorMatchingInlineSnapshot( + `"Missing required config value at 'secret' in 'mock-config'"`, + ); + expect(() => + handler.addOld(new ConfigReader({ secret: '' })), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'secret' in 'mock-config', got empty-string, wanted string"`, ); expect(() => handler.addOld(new ConfigReader({ secret: 'has spaces' })), - ).toThrow(/secret/); + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal secret, must be a valid base64 string"`, + ); expect(() => handler.addOld(new ConfigReader({ secret: 'hasnewline\n' })), - ).toThrow(/secret/); - expect(() => handler.addOld(new ConfigReader({ secret: 3 }))).toThrow( - /secret/, + ).toThrowErrorMatchingInlineSnapshot( + `"Illegal secret, must be a valid base64 string"`, + ); + expect(() => + handler.addOld(new ConfigReader({ secret: 3 })), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'secret' in 'mock-config', got number, wanted string"`, ); }); }); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts b/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts index 8448f295a6..9c60e70707 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/legacy.ts @@ -16,7 +16,8 @@ import { Config } from '@backstage/config'; import { base64url, decodeJwt, decodeProtectedHeader, jwtVerify } from 'jose'; -import { TokenHandler } from './types'; +import { readAccessRestrictionsFromConfig } from './helpers'; +import { AccessRestriptionsMap, TokenHandler } from './types'; /** * Handles `type: legacy` access. @@ -24,21 +25,38 @@ import { TokenHandler } from './types'; * @internal */ export class LegacyTokenHandler implements TokenHandler { - #entries: Array<{ key: Uint8Array; subject: string }> = []; + #entries = new Array<{ + key: Uint8Array; + result: { + subject: string; + allAccessRestrictions?: AccessRestriptionsMap; + }; + }>(); - add(options: Config) { - this.#doAdd(options.getString('secret'), options.getString('subject')); + add(config: Config) { + const allAccessRestrictions = readAccessRestrictionsFromConfig(config); + this.#doAdd( + config.getString('options.secret'), + config.getString('options.subject'), + allAccessRestrictions, + ); } // used only for the old backend.auth.keys array - addOld(options: Config) { + addOld(config: Config) { // This choice of subject is for compatibility reasons - this.#doAdd(options.getString('secret'), 'external:backstage-plugin'); + this.#doAdd(config.getString('secret'), 'external:backstage-plugin'); } - #doAdd(secret: string, subject: string) { + #doAdd( + secret: string, + subject: string, + allAccessRestrictions?: AccessRestriptionsMap, + ) { if (!secret.match(/^\S+$/)) { throw new Error('Illegal secret, must be a valid base64 string'); + } else if (!subject.match(/^\S+$/)) { + throw new Error('Illegal subject, must be a set of non-space characters'); } let key: Uint8Array; @@ -48,11 +66,19 @@ export class LegacyTokenHandler implements TokenHandler { throw new Error('Illegal secret, must be a valid base64 string'); } - if (!subject.match(/^\S+$/)) { - throw new Error('Illegal subject, must be a set of non-space characters'); + if (this.#entries.some(e => e.key === key)) { + throw new Error( + 'Legacy externalAccess token was declared more than once', + ); } - this.#entries.push({ key, subject }); + this.#entries.push({ + key, + result: { + subject, + allAccessRestrictions, + }, + }); } async verifyToken(token: string) { @@ -76,10 +102,10 @@ export class LegacyTokenHandler implements TokenHandler { return undefined; } - for (const entry of this.#entries) { + for (const { key, result } of this.#entries) { try { - await jwtVerify(token, entry.key); - return { subject: entry.subject }; + await jwtVerify(token, key); + return result; } catch (e) { if (e.code !== 'ERR_JWS_SIGNATURE_VERIFICATION_FAILED') { throw e; diff --git a/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts b/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts index 86bdf0bdee..a5945daba9 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/static.test.ts @@ -20,14 +20,36 @@ import { StaticTokenHandler } from './static'; describe('StaticTokenHandler', () => { it('accepts any of the added list of tokens', async () => { const handler = new StaticTokenHandler(); - handler.add(new ConfigReader({ token: 'abcabcabc', subject: 'one' })); - handler.add(new ConfigReader({ token: 'defdefdef', subject: 'two' })); + handler.add( + new ConfigReader({ + options: { token: 'abcabcabc', subject: 'one' }, + accessRestrictions: [{ plugin: 'scaffolder' }], + }), + ); + handler.add( + new ConfigReader({ + options: { token: 'defdefdef', subject: 'two' }, + accessRestrictions: [ + { plugin: 'catalog', permission: 'catalog.entity.read' }, + ], + }), + ); + const accessRestrictionsOne = new Map(Object.entries({ scaffolder: {} })); + const accessRestrictionsTwo = new Map( + Object.entries({ + catalog: { + permissionNames: ['catalog.entity.read'], + }, + }), + ); await expect(handler.verifyToken('abcabcabc')).resolves.toEqual({ subject: 'one', + allAccessRestrictions: accessRestrictionsOne, }); await expect(handler.verifyToken('defdefdef')).resolves.toEqual({ subject: 'two', + allAccessRestrictions: accessRestrictionsTwo, }); await expect(handler.verifyToken('ghighighi')).resolves.toBeUndefined(); }); @@ -41,71 +63,89 @@ describe('StaticTokenHandler', () => { const handler = new StaticTokenHandler(); expect(() => - handler.add(new ConfigReader({ _missingtoken: true, subject: 'ok' })), + handler.add( + new ConfigReader({ options: { _missingtoken: true, subject: 'ok' } }), + ), ).toThrowErrorMatchingInlineSnapshot( - `"Missing required config value at 'token' in 'mock-config'"`, + `"Missing required config value at 'options.token' in 'mock-config'"`, ); expect(() => - handler.add(new ConfigReader({ token: '', subject: 'ok' })), + handler.add(new ConfigReader({ options: { token: '', subject: 'ok' } })), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'token' in 'mock-config', got empty-string, wanted string"`, + `"Invalid type in config for key 'options.token' in 'mock-config', got empty-string, wanted string"`, ); expect(() => - handler.add(new ConfigReader({ token: 'has spaces', subject: 'ok' })), + handler.add( + new ConfigReader({ options: { token: 'has spaces', subject: 'ok' } }), + ), ).toThrowErrorMatchingInlineSnapshot( `"Illegal token, must be a set of non-space characters"`, ); expect(() => handler.add( new ConfigReader({ - token: 'hasnewlinebutislongenough\n', - subject: 'ok', + options: { + token: 'hasnewlinebutislongenough\n', + subject: 'ok', + }, }), ), ).toThrowErrorMatchingInlineSnapshot( `"Illegal token, must be a set of non-space characters"`, ); expect(() => - handler.add(new ConfigReader({ token: 'short', subject: 'ok' })), + handler.add( + new ConfigReader({ options: { token: 'short', subject: 'ok' } }), + ), ).toThrowErrorMatchingInlineSnapshot( `"Illegal token, must be at least 8 characters length"`, ); expect(() => - handler.add(new ConfigReader({ token: 3, subject: 'ok' })), + handler.add(new ConfigReader({ options: { token: 3, subject: 'ok' } })), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'token' in 'mock-config', got number, wanted string"`, + `"Invalid type in config for key 'options.token' in 'mock-config', got number, wanted string"`, ); expect(() => handler.add( - new ConfigReader({ token: 'validtoken', _missingsubject: true }), + new ConfigReader({ + options: { token: 'validtoken', _missingsubject: true }, + }), ), ).toThrowErrorMatchingInlineSnapshot( - `"Missing required config value at 'subject' in 'mock-config'"`, - ); - expect(() => - handler.add(new ConfigReader({ token: 'validtoken', subject: '' })), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'subject' in 'mock-config', got empty-string, wanted string"`, + `"Missing required config value at 'options.subject' in 'mock-config'"`, ); expect(() => handler.add( - new ConfigReader({ token: 'validtoken', subject: 'has spaces' }), + new ConfigReader({ options: { token: 'validtoken', subject: '' } }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid type in config for key 'options.subject' in 'mock-config', got empty-string, wanted string"`, + ); + expect(() => + handler.add( + new ConfigReader({ + options: { token: 'validtoken', subject: 'has spaces' }, + }), ), ).toThrowErrorMatchingInlineSnapshot( `"Illegal subject, must be a set of non-space characters"`, ); expect(() => handler.add( - new ConfigReader({ token: 'validtoken', subject: 'hasnewline\n' }), + new ConfigReader({ + options: { token: 'validtoken', subject: 'hasnewline\n' }, + }), ), ).toThrowErrorMatchingInlineSnapshot( `"Illegal subject, must be a set of non-space characters"`, ); expect(() => - handler.add(new ConfigReader({ token: 'validtoken', subject: 3 })), + handler.add( + new ConfigReader({ options: { token: 'validtoken', subject: 3 } }), + ), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid type in config for key 'subject' in 'mock-config', got number, wanted string"`, + `"Invalid type in config for key 'options.subject' in 'mock-config', got number, wanted string"`, ); }); }); diff --git a/packages/backend-app-api/src/services/implementations/auth/external/static.ts b/packages/backend-app-api/src/services/implementations/auth/external/static.ts index bae8e05f2b..1e89d8c6a1 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/static.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/static.ts @@ -15,7 +15,8 @@ */ import { Config } from '@backstage/config'; -import { TokenHandler } from './types'; +import { readAccessRestrictionsFromConfig } from './helpers'; +import { AccessRestriptionsMap, TokenHandler } from './types'; const MIN_TOKEN_LENGTH = 8; @@ -25,33 +26,37 @@ const MIN_TOKEN_LENGTH = 8; * @internal */ export class StaticTokenHandler implements TokenHandler { - #entries: Array<{ token: string; subject: string }> = []; + #entries = new Map< + string, + { + subject: string; + allAccessRestrictions?: AccessRestriptionsMap; + } + >(); + + add(config: Config) { + const token = config.getString('options.token'); + const subject = config.getString('options.subject'); + const allAccessRestrictions = readAccessRestrictionsFromConfig(config); - add(options: Config) { - const token = options.getString('token'); if (!token.match(/^\S+$/)) { throw new Error('Illegal token, must be a set of non-space characters'); - } - if (token.length < MIN_TOKEN_LENGTH) { + } else if (token.length < MIN_TOKEN_LENGTH) { throw new Error( `Illegal token, must be at least ${MIN_TOKEN_LENGTH} characters length`, ); - } - - const subject = options.getString('subject'); - if (!subject.match(/^\S+$/)) { + } else if (!subject.match(/^\S+$/)) { throw new Error('Illegal subject, must be a set of non-space characters'); + } else if (this.#entries.has(token)) { + throw new Error( + 'Static externalAccess token was declared more than once', + ); } - this.#entries.push({ token, subject }); + this.#entries.set(token, { subject, allAccessRestrictions }); } async verifyToken(token: string) { - const entry = this.#entries.find(e => e.token === token); - if (!entry) { - return undefined; - } - - return { subject: entry.subject }; + return this.#entries.get(token); } } diff --git a/packages/backend-app-api/src/services/implementations/auth/external/types.ts b/packages/backend-app-api/src/services/implementations/auth/external/types.ts index 5d33f09a22..6a1fd084ed 100644 --- a/packages/backend-app-api/src/services/implementations/auth/external/types.ts +++ b/packages/backend-app-api/src/services/implementations/auth/external/types.ts @@ -14,9 +14,21 @@ * limitations under the License. */ +import { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api'; import { Config } from '@backstage/config'; +export type AccessRestriptionsMap = Map< + string, // plugin ID + BackstagePrincipalAccessRestrictions +>; + export interface TokenHandler { add(options: Config): void; - verifyToken(token: string): Promise<{ subject: string } | undefined>; + verifyToken(token: string): Promise< + | { + subject: string; + allAccessRestrictions?: AccessRestriptionsMap; + } + | undefined + >; } diff --git a/packages/backend-app-api/src/services/implementations/auth/helpers.ts b/packages/backend-app-api/src/services/implementations/auth/helpers.ts index 01fb9537a1..eebe45eb76 100644 --- a/packages/backend-app-api/src/services/implementations/auth/helpers.ts +++ b/packages/backend-app-api/src/services/implementations/auth/helpers.ts @@ -17,6 +17,7 @@ import { BackstageCredentials, BackstageNonePrincipal, + BackstagePrincipalAccessRestrictions, BackstageServicePrincipal, BackstageUserPrincipal, } from '@backstage/backend-plugin-api'; @@ -25,6 +26,7 @@ import { InternalBackstageCredentials } from './types'; export function createCredentialsWithServicePrincipal( sub: string, token?: string, + accessRestrictions?: BackstagePrincipalAccessRestrictions, ): InternalBackstageCredentials { return { $$type: '@backstage/BackstageCredentials', @@ -33,6 +35,7 @@ export function createCredentialsWithServicePrincipal( principal: { type: 'service', subject: sub, + accessRestrictions, }, }; } diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index c65fc5cf3e..134e4b61da 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -16,6 +16,7 @@ import { isChildPath } from '@backstage/cli-common'; import { JsonObject } from '@backstage/types'; import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; +import { PermissionAttributes } from '@backstage/plugin-permission-common'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { QueryPermissionRequest } from '@backstage/plugin-permission-common'; import { QueryPermissionResponse } from '@backstage/plugin-permission-common'; @@ -136,6 +137,14 @@ export type BackstageNonePrincipal = { type: 'none'; }; +// @public +export type BackstagePrincipalAccessRestrictions = { + permissionNames?: string[]; + permissionAttributes?: { + action?: Array['action']>; + }; +}; + // @public (undocumented) export type BackstagePrincipalTypes = { user: BackstageUserPrincipal; @@ -148,6 +157,7 @@ export type BackstagePrincipalTypes = { export type BackstageServicePrincipal = { type: 'service'; subject: string; + accessRestrictions?: BackstagePrincipalAccessRestrictions; }; // @public (undocumented) diff --git a/packages/backend-plugin-api/src/services/definitions/AuthService.ts b/packages/backend-plugin-api/src/services/definitions/AuthService.ts index 827b4122cd..000ce8bbb1 100644 --- a/packages/backend-plugin-api/src/services/definitions/AuthService.ts +++ b/packages/backend-plugin-api/src/services/definitions/AuthService.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { PermissionAttributes } from '@backstage/plugin-permission-common'; import { JsonObject } from '@backstage/types'; /** @@ -40,6 +41,54 @@ export type BackstageServicePrincipal = { // Exact format TBD, possibly 'plugin:' or 'external:' subject: string; + + /** + * The access restrictions that apply to this principal. + * + * @remarks + * + * If no access restrictions are provided the principal is assumed to have + * unlimited access, at a framework level. The permissions system and + * individual plugins may or may not still apply additional access controls on + * top of this. + */ + accessRestrictions?: BackstagePrincipalAccessRestrictions; +}; + +/** + * The access restrictions that apply to a given principal. + * + * @public + */ +export type BackstagePrincipalAccessRestrictions = { + /** + * If given, the principal is limited to only performing actions with these + * named permissions. + * + * Note that this only applies where permissions checks are enabled in the + * first place. Endpoints that are not protected by the permissions system at + * all, are not affected by this setting. + * + * This array always has at least one element, or is missing entirely. + */ + permissionNames?: string[]; + /** + * If given, the principal is limited to only performing actions whose + * permissions have these attributes. + * + * Note that this only applies where permissions checks are enabled in the + * first place. Endpoints that are not protected by the permissions system at + * all, are not affected by this setting. + * + * This object always has at least one key, or is missing entirely. + */ + permissionAttributes?: { + /** + * Match any of these action values. This array always has at least one + * element, or is missing entirely. + */ + action?: Array['action']>; + }; }; /** diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index 5739add90c..f22213a285 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -20,6 +20,7 @@ export type { BackstageCredentials, BackstageUserPrincipal, BackstageServicePrincipal, + BackstagePrincipalAccessRestrictions, BackstagePrincipalTypes, BackstageNonePrincipal, } from './AuthService'; diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 4f6eb47243..5a192cb61f 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -11,6 +11,7 @@ import { Backend } from '@backstage/backend-app-api'; import { BackendFeature } from '@backstage/backend-plugin-api'; import { BackstageCredentials } from '@backstage/backend-plugin-api'; import { BackstageNonePrincipal } from '@backstage/backend-plugin-api'; +import { BackstagePrincipalAccessRestrictions } 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'; @@ -68,6 +69,7 @@ export namespace mockCredentials { } export function service( subject?: string, + accessRestrictions?: BackstagePrincipalAccessRestrictions, ): BackstageCredentials; export namespace service { export function header(options?: TokenOptions): string; diff --git a/packages/backend-test-utils/src/next/services/mockCredentials.test.ts b/packages/backend-test-utils/src/next/services/mockCredentials.test.ts index ed0071d4b8..343d6d6d20 100644 --- a/packages/backend-test-utils/src/next/services/mockCredentials.test.ts +++ b/packages/backend-test-utils/src/next/services/mockCredentials.test.ts @@ -134,6 +134,16 @@ describe('mockCredentials', () => { expect(mockCredentials.service.invalidHeader()).toBe( 'Bearer mock-invalid-service-token', ); + expect( + mockCredentials.service('test', { permissionNames: ['do.it'] }), + ).toEqual({ + $$type: '@backstage/BackstageCredentials', + principal: { + type: 'service', + subject: 'test', + accessRestrictions: { permissionNames: ['do.it'] }, + }, + }); }); it('should throw on invalid user entity refs', () => { diff --git a/packages/backend-test-utils/src/next/services/mockCredentials.ts b/packages/backend-test-utils/src/next/services/mockCredentials.ts index 16d2381c73..4214320142 100644 --- a/packages/backend-test-utils/src/next/services/mockCredentials.ts +++ b/packages/backend-test-utils/src/next/services/mockCredentials.ts @@ -17,6 +17,7 @@ import { BackstageCredentials, BackstageNonePrincipal, + BackstagePrincipalAccessRestrictions, BackstageServicePrincipal, BackstageUserPrincipal, } from '@backstage/backend-plugin-api'; @@ -202,14 +203,19 @@ export namespace mockCredentials { /** * Creates a mocked credentials object for a service principal. * - * The default subject is 'external:test-service'. + * The default subject is 'external:test-service', and no access restrictions. */ export function service( subject: string = DEFAULT_MOCK_SERVICE_SUBJECT, + accessRestrictions?: BackstagePrincipalAccessRestrictions, ): BackstageCredentials { return { $$type: '@backstage/BackstageCredentials', - principal: { type: 'service', subject }, + principal: { + type: 'service', + subject, + ...(accessRestrictions ? { accessRestrictions } : {}), + }, }; } diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index 215f624fe9..8273606845 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -48,7 +48,9 @@ const discovery: PluginEndpointDiscovery = { }; const testBasicPermission = createPermission({ name: 'test.permission', - attributes: {}, + attributes: { + action: 'create', + }, }); const testResourcePermission = createPermission({ @@ -362,4 +364,244 @@ describe('ServerPermissionClient', () => { }); }); }); + + describe('with access restrictions', () => { + it.each([{ enabled: true }, { enabled: false }])( + 'short circuits the response when using a service principal, applying the relevant access restrictions if present, when permissions %p', + async permissionConfig => { + const client = ServerPermissionClient.fromConfig( + new ConfigReader({ + permission: permissionConfig, + }), + { + discovery, + tokenManager: mockServices.tokenManager(), + auth: mockServices.auth(), + }, + ); + + // no restrictions for the given plugin + await expect( + client.authorize( + [ + { + permission: createPermission({ + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', {}), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.ALLOW }]); + await expect( + client.authorizeConditional( + [ + { + resourceRef: undefined as any, + permission: createPermission({ + resourceType: 'test', + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', {}), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.ALLOW }]); + + // matching permission name + await expect( + client.authorize( + [ + { + permission: createPermission({ + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionNames: ['test.permission', 'other'], + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.ALLOW }]); + await expect( + client.authorizeConditional( + [ + { + resourceRef: undefined as any, + permission: createPermission({ + resourceType: 'test', + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionNames: ['test.permission', 'other'], + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.ALLOW }]); + + // matching attributes + await expect( + client.authorize( + [ + { + permission: createPermission({ + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionAttributes: { + action: ['create', 'other' as any], + }, + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.ALLOW }]); + await expect( + client.authorizeConditional( + [ + { + resourceRef: undefined as any, + permission: createPermission({ + resourceType: 'test', + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionAttributes: { + action: ['create', 'other' as any], + }, + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.ALLOW }]); + + // matching permission name but not attributes + await expect( + client.authorize( + [ + { + permission: createPermission({ + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionNames: ['test.permission'], + permissionAttributes: { + action: ['other' as any], + }, + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.DENY }]); + await expect( + client.authorizeConditional( + [ + { + resourceRef: undefined as any, + permission: createPermission({ + resourceType: 'test', + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionNames: ['test.permission'], + permissionAttributes: { + action: ['other' as any], + }, + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.DENY }]); + + // matching attributes but not permission name + await expect( + client.authorize( + [ + { + permission: createPermission({ + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionNames: ['wrong-name'], + permissionAttributes: { + action: ['create'], + }, + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.DENY }]); + await expect( + client.authorizeConditional( + [ + { + resourceRef: undefined as any, + permission: createPermission({ + resourceType: 'test', + name: 'test.permission', + attributes: { + action: 'create', + }, + }), + }, + ], + { + credentials: mockCredentials.service('foo', { + permissionNames: ['wrong-name'], + permissionAttributes: { + action: ['create'], + }, + }), + }, + ), + ).resolves.toEqual([{ result: AuthorizeResult.DENY }]); + }, + ); + }); }); diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index f8570fee74..cb39f310bd 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -21,16 +21,19 @@ import { import { AuthService, BackstageCredentials, + BackstageServicePrincipal, DiscoveryService, PermissionsService, PermissionsServiceRequestOptions, } from '@backstage/backend-plugin-api'; import { Config } from '@backstage/config'; import { - AuthorizeResult, - PermissionClient, AuthorizePermissionRequest, AuthorizePermissionResponse, + AuthorizeResult, + DefinitivePolicyDecision, + Permission, + PermissionClient, PolicyDecision, QueryPermissionRequest, } from '@backstage/plugin-permission-common'; @@ -91,31 +94,39 @@ export class ServerPermissionClient implements PermissionsService { queries: QueryPermissionRequest[], options?: PermissionsServiceRequestOptions, ): Promise { - if (await this.#shouldPermissionsBeApplied(options)) { - return this.#permissionClient.authorizeConditional( - queries, - await this.#getRequestOptions(options), - ); + const credentials = await this.#getIncomingCredentials(options); + if (credentials && this.#auth.isPrincipal(credentials, 'service')) { + return this.#servicePrincipalDecision(queries, credentials); + } else if (!this.#permissionEnabled) { + return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); } - return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); + return this.#permissionClient.authorizeConditional( + queries, + await this.#getRequestOptions(options), + ); } async authorize( requests: AuthorizePermissionRequest[], options?: PermissionsServiceRequestOptions, ): Promise { - if (await this.#shouldPermissionsBeApplied(options)) { - return this.#permissionClient.authorize( - requests, - await this.#getRequestOptions(options), - ); + const credentials = await this.#getIncomingCredentials(options); + if (credentials && this.#auth.isPrincipal(credentials, 'service')) { + return this.#servicePrincipalDecision(requests, credentials); + } else if (!this.#permissionEnabled) { + return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); } - return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); + return this.#permissionClient.authorize( + requests, + await this.#getRequestOptions(options), + ); } - async #getRequestOptions(options?: PermissionsServiceRequestOptions) { + async #getRequestOptions( + options?: PermissionsServiceRequestOptions, + ): Promise<{ token?: string } | undefined> { if (options && 'credentials' in options) { if (this.#auth.isPrincipal(options.credentials, 'none')) { return {}; @@ -130,30 +141,48 @@ export class ServerPermissionClient implements PermissionsService { return options; } - async #shouldPermissionsBeApplied( + async #getIncomingCredentials( options?: PermissionsServiceRequestOptions, - ) { - if (!this.#permissionEnabled) { - return false; - } - - let credentials: BackstageCredentials; + ): Promise { if (options && 'credentials' in options) { - credentials = options.credentials; - } else { - if (!options?.token) { - return true; - } + return options.credentials; + } + + if (options?.token) { try { - credentials = await this.#auth.authenticate(options.token); + return await this.#auth.authenticate(options.token); } catch { - return true; + // ignore } } - if (this.#auth.isPrincipal(credentials, 'service')) { - return false; - } - return true; + return undefined; + } + + /** + * For service principals, we can always make an immediate definitive decision + * based on their associated access restrictions (if any). + */ + #servicePrincipalDecision( + input: { permission: Permission }[], + credentials: BackstageCredentials, + ): DefinitivePolicyDecision[] { + const { permissionNames, permissionAttributes } = + credentials.principal.accessRestrictions ?? {}; + + return input.map(item => { + if (permissionNames && !permissionNames.includes(item.permission.name)) { + return { result: AuthorizeResult.DENY }; + } + + if (permissionAttributes?.action) { + const action = item.permission.attributes?.action; + if (!action || !permissionAttributes.action.includes(action)) { + return { result: AuthorizeResult.DENY }; + } + } + + return { result: AuthorizeResult.ALLOW }; + }); } }