Skip to content

Commit 4ecd9f0

Browse files
authored
fix(orchestrator):Limit access to workflow instances to initiators only (#867)
Signed-off-by: Lior Soffer <liorsoffer1@gmail.com>
1 parent 4265f44 commit 4ecd9f0

17 files changed

Lines changed: 442 additions & 305 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-orchestrator-backend': patch
3+
'@red-hat-developer-hub/backstage-plugin-orchestrator-common': patch
4+
---
5+
6+
Limit access to workflow instances to initiators only

workspaces/orchestrator/docs/rbac-policy.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ p, role:default/workflowUser, orchestrator.workflow.use.yamlgreet, update, allow
88
p, role:default/workflowAdmin, orchestrator.workflow, read, allow
99
p, role:default/workflowAdmin, orchestrator.workflow.use, update, allow
1010
p, role:default/workflowAdmin, orchestrator.workflowAdminView, read, allow
11+
p, role:default/workflowAdmin, orchestrator.instanceAdminView, read, allow
1112

1213
p, role:default/workflowDenied, orchestrator.workflow, read, deny
1314
p, role:default/workflowDenied, orchestrator.workflow.use, update, deny

workspaces/orchestrator/plugins/orchestrator-backend/src/plugin.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const orchestratorPlugin = createBackendPlugin({
4141
scheduler: coreServices.scheduler,
4242
httpAuth: coreServices.httpAuth,
4343
http: coreServices.httpRouter,
44+
userInfo: coreServices.userInfo,
4445
},
4546
async init(props) {
4647
const { http } = props;

workspaces/orchestrator/plugins/orchestrator-backend/src/routerWrapper/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type {
2222
PermissionsService,
2323
SchedulerService,
2424
UrlReaderService,
25+
UserInfoService,
2526
} from '@backstage/backend-plugin-api';
2627
import type { CatalogApi } from '@backstage/catalog-client';
2728
import type { Config } from '@backstage/config';
@@ -41,6 +42,7 @@ export interface RouterOptions {
4142
scheduler: SchedulerService;
4243
permissions: PermissionsService;
4344
httpAuth: HttpAuthService;
45+
userInfo: UserInfoService;
4446
}
4547

4648
export async function createRouter(
@@ -71,5 +73,6 @@ export async function createRouter(
7173
scheduler: args.scheduler,
7274
permissions: args.permissions,
7375
httpAuth: args.httpAuth,
76+
userInfo: args.userInfo,
7477
});
7578
}

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/mapping/V2Mappings.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export function mapToProcessInstanceDTO(
144144
duration: duration,
145145
// @ts-ignore
146146
workflowdata: variables?.workflowdata,
147+
initiatorEntity: variables?.initiatorEntity as string,
147148
state: processInstance.state
148149
? getProcessInstancesStatusDTOFromString(processInstance.state)
149150
: undefined,

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/v2.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ describe('executeWorkflow', () => {
382382
workflowData,
383383
workflowInfo.id,
384384
'businessKey',
385+
'someUserEntity',
385386
);
386387

387388
// Assert

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/v2.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import {
2525
ProcessInstance,
2626
ProcessInstanceListResultDTO,
2727
ProcessInstanceState,
28-
ProcessInstanceVariables,
2928
WorkflowDTO,
3029
WorkflowInfo,
3130
WorkflowOverviewDTO,
@@ -158,6 +157,7 @@ export class V2 {
158157
executeWorkflowRequestDTO: ExecuteWorkflowRequestDTO,
159158
workflowId: string,
160159
businessKey: string | undefined,
160+
initiatorEntity: string,
161161
): Promise<ExecuteWorkflowResponseDTO> {
162162
const definition = await this.orchestratorService.fetchWorkflowInfo({
163163
definitionId: workflowId,
@@ -170,8 +170,10 @@ export class V2 {
170170
}
171171
const executionResponse = await this.orchestratorService.executeWorkflow({
172172
definitionId: workflowId,
173-
inputData:
174-
executeWorkflowRequestDTO.inputData as ProcessInstanceVariables,
173+
inputData: {
174+
workflowdata: executeWorkflowRequestDTO.inputData,
175+
initiatorEntity: initiatorEntity,
176+
},
175177
authTokens: executeWorkflowRequestDTO.authTokens as Array<AuthToken>,
176178
serviceUrl: definition.serviceUrl,
177179
businessKey,

workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
LoggerService,
2323
PermissionsService,
2424
SchedulerService,
25+
UserInfoService,
2526
} from '@backstage/backend-plugin-api';
2627
import type { Config } from '@backstage/config';
2728
import type { DiscoveryApi } from '@backstage/core-plugin-api';
@@ -41,8 +42,11 @@ import { Request as HttpRequest } from 'express-serve-static-core';
4142
import { OpenAPIBackend, Request } from 'openapi-backend';
4243

4344
import {
45+
FieldFilter,
4446
Filter,
47+
NestedFilter,
4548
openApiDocument,
49+
orchestratorInstanceAdminViewPermission,
4650
orchestratorPermissions,
4751
orchestratorWorkflowPermission,
4852
orchestratorWorkflowSpecificPermission,
@@ -100,6 +104,20 @@ const authorize = async (
100104
);
101105
};
102106

107+
const isUserAuthorizedForInstanceAdminViewPermission = async (
108+
request: HttpRequest,
109+
permissionsSvc: PermissionsService,
110+
httpAuth: HttpAuthService,
111+
): Promise<boolean> => {
112+
const credentials = await httpAuth.credentials(request);
113+
const [decision] = await permissionsSvc.authorize(
114+
[{ permission: orchestratorInstanceAdminViewPermission }],
115+
{ credentials },
116+
);
117+
118+
return decision.result === AuthorizeResult.ALLOW;
119+
};
120+
103121
const filterAuthorizedWorkflowIds = async (
104122
request: HttpRequest,
105123
permissionsSvc: PermissionsService,
@@ -173,6 +191,7 @@ export async function createBackendRouter(
173191
scheduler,
174192
permissions,
175193
httpAuth,
194+
userInfo,
176195
} = options;
177196
const publicServices = initPublicServices(logger, config, scheduler);
178197

@@ -203,6 +222,7 @@ export async function createBackendRouter(
203222
permissions,
204223
httpAuth,
205224
auditor,
225+
userInfo,
206226
);
207227
setupExternalRoutes(router, discovery, scaffolderService, auditor);
208228

@@ -313,6 +333,7 @@ function setupInternalRoutes(
313333
permissions: PermissionsService,
314334
httpAuth: HttpAuthService,
315335
auditor: AuditorService,
336+
userInfo: UserInfoService,
316337
) {
317338
function manageDenyAuthorization(auditEvent: AuditorServiceEvent) {
318339
const error = new UnauthorizedError();
@@ -404,6 +425,10 @@ function setupInternalRoutes(
404425
'executeWorkflow',
405426
async (c, req: express.Request, res: express.Response, next) => {
406427
const workflowId = c.request.params.workflowId as string;
428+
const credentials = await httpAuth.credentials(req);
429+
const initiatorEntity = await (
430+
await userInfo.getUserInfo(credentials)
431+
).userEntityRef;
407432

408433
const auditEvent = await auditor.createEvent({
409434
eventId: 'execute-workflow',
@@ -434,7 +459,12 @@ function setupInternalRoutes(
434459
const executeWorkflowRequestDTO = req.body;
435460

436461
return routerApi.v2
437-
.executeWorkflow(executeWorkflowRequestDTO, workflowId, businessKey)
462+
.executeWorkflow(
463+
executeWorkflowRequestDTO,
464+
workflowId,
465+
businessKey,
466+
initiatorEntity,
467+
)
438468
.then(result => {
439469
auditEvent.success({ meta: { id: result.id } });
440470
return res.status(200).json(result);
@@ -778,9 +808,46 @@ function setupInternalRoutes(
778808
if (!authorizedWorkflowIds || authorizedWorkflowIds.length === 0)
779809
res.json([]);
780810

811+
const credentials = await httpAuth.credentials(req);
812+
const initiatorEntity = (await userInfo.getUserInfo(credentials))
813+
.userEntityRef;
814+
const isUserAuthorizedForInstanceAdminView: boolean = // This permission will let user see ALL instances (including ones others created)
815+
await isUserAuthorizedForInstanceAdminViewPermission(
816+
req,
817+
permissions,
818+
httpAuth,
819+
);
820+
821+
const requestFilters = getRequestFilters(req);
822+
823+
let filters = requestFilters;
824+
825+
if (!isUserAuthorizedForInstanceAdminView) {
826+
const initiatorEntityFilter: FieldFilter = {
827+
operator: 'EQ',
828+
value: initiatorEntity,
829+
field: 'initiatorEntity',
830+
};
831+
832+
const nestedVariablesFilter: NestedFilter = {
833+
field: 'variables',
834+
nested: initiatorEntityFilter,
835+
};
836+
837+
if (requestFilters === undefined) {
838+
filters = nestedVariablesFilter;
839+
} else {
840+
// combine filters
841+
filters = {
842+
operator: 'AND',
843+
filters: [nestedVariablesFilter, requestFilters],
844+
};
845+
}
846+
}
847+
781848
const result = await routerApi.v2.getInstances(
782849
buildPagination(req),
783-
getRequestFilters(req),
850+
filters,
784851
authorizedWorkflowIds,
785852
);
786853

@@ -834,6 +901,28 @@ function setupInternalRoutes(
834901
manageDenyAuthorization(auditEvent);
835902
}
836903

904+
const credentials = await httpAuth.credentials(request);
905+
const initiatorEntity = (await userInfo.getUserInfo(credentials))
906+
.userEntityRef;
907+
// Check if user is authorized to view all instances
908+
const isUserAuthorizedForInstanceAdminView =
909+
await isUserAuthorizedForInstanceAdminViewPermission(
910+
request,
911+
permissions,
912+
httpAuth,
913+
);
914+
915+
// If not an admin, enforce initiatorEntity check
916+
if (!isUserAuthorizedForInstanceAdminView) {
917+
const instanceInitiatorEntity =
918+
assessedInstance.instance.initiatorEntity;
919+
if (instanceInitiatorEntity !== initiatorEntity) {
920+
throw new Error(
921+
`Unauthorized to access instance ${instanceId} not initiated by user.`,
922+
);
923+
}
924+
}
925+
837926
auditEvent.success();
838927
res.status(200).json(assessedInstance);
839928
} catch (error) {

0 commit comments

Comments
 (0)