diff --git a/.changeset/cold-rocks-laugh.md b/.changeset/cold-rocks-laugh.md new file mode 100644 index 0000000000..5686e94707 --- /dev/null +++ b/.changeset/cold-rocks-laugh.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-backend': minor +--- + +Improved validation for the `/authorize` endpoint when a `resourceRef` is provided alongside a basic permission. Additionally, introduced a clearer error message for cases where users attempt to directly evaluate conditional permissions. diff --git a/.changeset/pink-geese-teach.md b/.changeset/pink-geese-teach.md new file mode 100644 index 0000000000..0264fe1ec6 --- /dev/null +++ b/.changeset/pink-geese-teach.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-defaults': minor +'@backstage/backend-plugin-api': minor +'@backstage/backend-test-utils': minor +--- + +Added `actor` property to `BackstageUserPrincipal` containing the subject of the last service (if any) who performed authentication on behalf of the user. diff --git a/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts b/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts index a81a99fec7..189976ed7d 100644 --- a/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts +++ b/packages/backend-defaults/src/entrypoints/auth/DefaultAuthService.ts @@ -68,6 +68,7 @@ export class DefaultAuthService implements AuthService { userResult.userEntityRef, pluginResult.limitedUserToken, this.#getJwtExpiration(pluginResult.limitedUserToken), + pluginResult.subject, ); } return createCredentialsWithServicePrincipal(pluginResult.subject); diff --git a/packages/backend-defaults/src/entrypoints/auth/helpers.ts b/packages/backend-defaults/src/entrypoints/auth/helpers.ts index dad36670b9..58e2bcff74 100644 --- a/packages/backend-defaults/src/entrypoints/auth/helpers.ts +++ b/packages/backend-defaults/src/entrypoints/auth/helpers.ts @@ -51,6 +51,7 @@ export function createCredentialsWithUserPrincipal( sub: string, token: string, expiresAt?: Date, + actor?: string, ): InternalBackstageCredentials { return Object.defineProperty( { @@ -60,6 +61,9 @@ export function createCredentialsWithUserPrincipal( principal: { type: 'user', userEntityRef: sub, + ...(actor && { + actor: { type: 'service', subject: actor }, + }), }, }, 'token', diff --git a/packages/backend-defaults/src/entrypoints/permissionsRegistry/permissionsRegistryServiceFactory.ts b/packages/backend-defaults/src/entrypoints/permissionsRegistry/permissionsRegistryServiceFactory.ts index 7ff6eabf03..f599c4906a 100644 --- a/packages/backend-defaults/src/entrypoints/permissionsRegistry/permissionsRegistryServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/permissionsRegistry/permissionsRegistryServiceFactory.ts @@ -23,6 +23,8 @@ import { PermissionResourceRef, createPermissionIntegrationRouter, } from '@backstage/plugin-permission-node'; +import { NotAllowedError } from '@backstage/errors'; +import Router from 'express-promise-router'; function assertRefPluginId(ref: PermissionResourceRef, pluginId: string) { if (ref.pluginId !== pluginId) { @@ -44,14 +46,34 @@ function assertRefPluginId(ref: PermissionResourceRef, pluginId: string) { export const permissionsRegistryServiceFactory = createServiceFactory({ service: coreServices.permissionsRegistry, deps: { + auth: coreServices.auth, + httpAuth: coreServices.httpAuth, lifecycle: coreServices.lifecycle, httpRouter: coreServices.httpRouter, pluginMetadata: coreServices.pluginMetadata, }, - async factory({ httpRouter, lifecycle, pluginMetadata }) { + async factory({ auth, httpAuth, httpRouter, lifecycle, pluginMetadata }) { const router = createPermissionIntegrationRouter(); + const pluginId = pluginMetadata.getId(); + const applyConditionMiddleware = Router(); + applyConditionMiddleware.use( + '/.well-known/backstage/permissions/apply-conditions', + async (req, _res, next) => { + const credentials = await httpAuth.credentials(req, { + allow: ['user', 'service'], + }); + if ( + auth.isPrincipal(credentials, 'user') && + !credentials.principal.actor + ) { + throw new NotAllowedError(); + } + next(); + }, + ); + httpRouter.use(applyConditionMiddleware); httpRouter.use(router); let started = false; diff --git a/packages/backend-plugin-api/report.api.md b/packages/backend-plugin-api/report.api.md index 30678f124f..52843dec96 100644 --- a/packages/backend-plugin-api/report.api.md +++ b/packages/backend-plugin-api/report.api.md @@ -177,6 +177,7 @@ export interface BackstageUserInfo { export type BackstageUserPrincipal = { type: 'user'; userEntityRef: string; + actor?: BackstageServicePrincipal; }; // @public diff --git a/packages/backend-plugin-api/src/services/definitions/AuthService.ts b/packages/backend-plugin-api/src/services/definitions/AuthService.ts index 1e0cf6e56b..c4e03bb4b7 100644 --- a/packages/backend-plugin-api/src/services/definitions/AuthService.ts +++ b/packages/backend-plugin-api/src/services/definitions/AuthService.ts @@ -35,6 +35,17 @@ export type BackstageUserPrincipal = { * The entity ref of the user entity that this principal represents. */ userEntityRef: string; + + /** + * The service principal that issued the token on behalf of the user. + * + * @remarks + * + * This field is present in scenarios where a backend service acts on behalf + * of a user. It provides context about the intermediary service that + * facilitated the authentication. + */ + actor?: BackstageServicePrincipal; }; /** diff --git a/packages/backend-test-utils/report.api.md b/packages/backend-test-utils/report.api.md index 89035cbaec..5497497068 100644 --- a/packages/backend-test-utils/report.api.md +++ b/packages/backend-test-utils/report.api.md @@ -88,6 +88,11 @@ export namespace mockCredentials { } export function user( userEntityRef?: string, + options?: { + actor?: { + subject: string; + }; + }, ): BackstageCredentials; export namespace user { export function header(userEntityRef?: string): string; @@ -95,7 +100,14 @@ export namespace mockCredentials { export function invalidHeader(): string; // (undocumented) export function invalidToken(): string; - export function token(userEntityRef?: string): string; + export function token( + userEntityRef?: string, + options?: { + actor?: { + subject: string; + }; + }, + ): string; } } diff --git a/packages/backend-test-utils/src/next/services/MockAuthService.ts b/packages/backend-test-utils/src/next/services/MockAuthService.ts index 4bce87bd2a..660ff61a57 100644 --- a/packages/backend-test-utils/src/next/services/MockAuthService.ts +++ b/packages/backend-test-utils/src/next/services/MockAuthService.ts @@ -73,11 +73,11 @@ export class MockAuthService implements AuthService { } if (token.startsWith(MOCK_USER_TOKEN_PREFIX)) { - const { sub: userEntityRef }: UserTokenPayload = JSON.parse( + const { sub: userEntityRef, actor }: UserTokenPayload = JSON.parse( token.slice(MOCK_USER_TOKEN_PREFIX.length), ); - return mockCredentials.user(userEntityRef); + return mockCredentials.user(userEntityRef, { actor }); } if (token.startsWith(MOCK_USER_LIMITED_TOKEN_PREFIX)) { diff --git a/packages/backend-test-utils/src/next/services/mockCredentials.ts b/packages/backend-test-utils/src/next/services/mockCredentials.ts index 4214320142..231c108ac4 100644 --- a/packages/backend-test-utils/src/next/services/mockCredentials.ts +++ b/packages/backend-test-utils/src/next/services/mockCredentials.ts @@ -55,6 +55,7 @@ function validateUserEntityRef(ref: string) { */ export type UserTokenPayload = { sub?: string; + actor?: { subject: string }; }; /** @@ -107,11 +108,18 @@ export namespace mockCredentials { */ export function user( userEntityRef: string = DEFAULT_MOCK_USER_ENTITY_REF, + options?: { actor?: { subject: string } }, ): BackstageCredentials { validateUserEntityRef(userEntityRef); return { $$type: '@backstage/BackstageCredentials', - principal: { type: 'user', userEntityRef }, + principal: { + type: 'user', + userEntityRef, + ...(options?.actor && { + actor: { type: 'service', subject: options.actor.subject }, + }), + }, }; } @@ -124,11 +132,17 @@ export namespace mockCredentials { * into the token and forwarded to the credentials object when authenticated * by the mock auth service. */ - export function token(userEntityRef?: string): string { + export function token( + userEntityRef?: string, + options?: { actor?: { subject: string } }, + ): string { if (userEntityRef) { validateUserEntityRef(userEntityRef); return `${MOCK_USER_TOKEN_PREFIX}${JSON.stringify({ sub: userEntityRef, + ...(options?.actor && { + actor: { subject: options.actor.subject }, + }), } satisfies UserTokenPayload)}`; } return MOCK_USER_TOKEN; diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index e3fb5418cf..9c8fa4ad5f 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -217,6 +217,9 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') + .auth(userTokenIssuedByService(), { + type: 'bearer', + }) .send({ items: [ { @@ -545,7 +548,7 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .auth(mockCredentials.user.token(), { type: 'bearer' }) + .auth(userTokenIssuedByService(), { type: 'bearer' }) .send({ items: [ { @@ -592,7 +595,9 @@ describe('createRouter', () => { expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', - mockCredentials.user(), + mockCredentials.user('user:default/spiderman', { + actor: { subject: 'some-service' }, + }), [ expect.objectContaining({ id: '123', @@ -605,7 +610,9 @@ describe('createRouter', () => { expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-2', - mockCredentials.user(), + mockCredentials.user('user:default/spiderman', { + actor: { subject: 'some-service' }, + }), [ expect.objectContaining({ id: '234', @@ -719,6 +726,12 @@ describe('createRouter', () => { }); }, ); + + function userTokenIssuedByService() { + return mockCredentials.user.token('user:default/spiderman', { + actor: { subject: 'some-service' }, + }); + } }); it.each([ @@ -761,6 +774,35 @@ describe('createRouter', () => { { id: '123', permission: { attributes: { invalid: 'attribute' } } }, ], }, + { + items: [ + { + id: '123', + // basic permission can't have resourceRef + resourceRef: 'resource:1', + permission: { + type: 'basic', + name: 'test.permission', + attributes: {}, + }, + }, + ], + }, + { + items: [ + { + id: '123', + // resource ref should be a string + resourceRef: ['resource:1'], + permission: { + type: 'resource', + name: 'test.permission', + attributes: {}, + resourceType: 'test-resource-1', + }, + }, + ], + }, ])('returns a 400 error for invalid request %#', async requestBody => { const response = await request(app).post('/authorize').send(requestBody); @@ -794,6 +836,7 @@ describe('createRouter', () => { resourceType: 'test-resource-1', attributes: {}, }, + resourceRef: 'resource:1', }, ], }); @@ -807,5 +850,41 @@ describe('createRouter', () => { }), ); }); + + it(`returns a 400 error if the request doesn't contain resourceRef for credentials not issued by a service`, async () => { + policy.handle.mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'test-plugin', + resourceType: 'test-resource-2', + conditions: {}, + }); + + const response = await request(app) + .post('/authorize') + .send({ + items: [ + { + id: '123', + permission: { + type: 'resource', + name: 'test.permission', + resourceType: 'test-resource-1', + attributes: {}, + }, + }, + ], + }); + + expect(response.status).toEqual(400); + expect(response.body).toEqual( + expect.objectContaining({ + error: expect.objectContaining({ + message: expect.stringMatching( + /Resource permissions require a resourceRef to be set/i, + ), + }), + }), + ); + }); }); }); diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index c624947375..83dc90a363 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -62,27 +62,33 @@ const attributesSchema: z.ZodSchema = z.object({ .optional(), }); -const permissionSchema = z.union([ - z.object({ - type: z.literal('basic'), - name: z.string(), - attributes: attributesSchema, - }), - z.object({ - type: z.literal('resource'), - name: z.string(), - attributes: attributesSchema, - resourceType: z.string(), - }), -]); +const basicPermissionSchema = z.object({ + type: z.literal('basic'), + name: z.string(), + attributes: attributesSchema, +}); + +const resourcePermissionSchema = z.object({ + type: z.literal('resource'), + name: z.string(), + attributes: attributesSchema, + resourceType: z.string(), +}); const evaluatePermissionRequestSchema: z.ZodSchema< IdentifiedPermissionMessage -> = z.object({ - id: z.string(), - resourceRef: z.string().optional(), - permission: permissionSchema, -}); +> = z.union([ + z.object({ + id: z.string(), + resourceRef: z.undefined().optional(), + permission: basicPermissionSchema, + }), + z.object({ + id: z.string(), + resourceRef: z.string().optional(), + permission: resourcePermissionSchema, + }), +]); const evaluatePermissionRequestBatchSchema: z.ZodSchema = z.object({ @@ -203,6 +209,11 @@ export async function createRouter( ); } + const disabledDefaultAuthPolicy = + config.getOptionalBoolean( + 'backend.auth.dangerouslyDisableDefaultAuthPolicy', + ) ?? false; + const permissionIntegrationClient = new PermissionIntegrationClient({ discovery, auth, @@ -235,6 +246,22 @@ export async function createRouter( const body = parseResult.data; + if ( + (auth.isPrincipal(credentials, 'none') && !disabledDefaultAuthPolicy) || + (auth.isPrincipal(credentials, 'user') && !credentials.principal.actor) + ) { + if ( + body.items.some( + r => + isResourcePermission(r.permission) && r.resourceRef === undefined, + ) + ) { + throw new InputError( + 'Resource permissions require a resourceRef to be set. Direct user requests without a resourceRef are not allowed.', + ); + } + } + res.json({ items: await handleRequest( body.items,