Merge pull request #29615 from backstage/permission-framework-tweaks
Add actor to user principal and improve validation in permission-backend
This commit is contained in:
@@ -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.
|
||||
@@ -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.
|
||||
@@ -68,6 +68,7 @@ export class DefaultAuthService implements AuthService {
|
||||
userResult.userEntityRef,
|
||||
pluginResult.limitedUserToken,
|
||||
this.#getJwtExpiration(pluginResult.limitedUserToken),
|
||||
pluginResult.subject,
|
||||
);
|
||||
}
|
||||
return createCredentialsWithServicePrincipal(pluginResult.subject);
|
||||
|
||||
@@ -51,6 +51,7 @@ export function createCredentialsWithUserPrincipal(
|
||||
sub: string,
|
||||
token: string,
|
||||
expiresAt?: Date,
|
||||
actor?: string,
|
||||
): InternalBackstageCredentials<BackstageUserPrincipal> {
|
||||
return Object.defineProperty(
|
||||
{
|
||||
@@ -60,6 +61,9 @@ export function createCredentialsWithUserPrincipal(
|
||||
principal: {
|
||||
type: 'user',
|
||||
userEntityRef: sub,
|
||||
...(actor && {
|
||||
actor: { type: 'service', subject: actor },
|
||||
}),
|
||||
},
|
||||
},
|
||||
'token',
|
||||
|
||||
+23
-1
@@ -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;
|
||||
|
||||
@@ -177,6 +177,7 @@ export interface BackstageUserInfo {
|
||||
export type BackstageUserPrincipal = {
|
||||
type: 'user';
|
||||
userEntityRef: string;
|
||||
actor?: BackstageServicePrincipal;
|
||||
};
|
||||
|
||||
// @public
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -88,6 +88,11 @@ export namespace mockCredentials {
|
||||
}
|
||||
export function user(
|
||||
userEntityRef?: string,
|
||||
options?: {
|
||||
actor?: {
|
||||
subject: string;
|
||||
};
|
||||
},
|
||||
): BackstageCredentials<BackstageUserPrincipal>;
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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<BackstageUserPrincipal> {
|
||||
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;
|
||||
|
||||
@@ -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,
|
||||
),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -62,27 +62,33 @@ const attributesSchema: z.ZodSchema<PermissionAttributes> = 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<EvaluatePermissionRequest>
|
||||
> = 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<EvaluatePermissionRequestBatch> =
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user