From ae79d51d87d79b415768e79aeb392ad9d574e820 Mon Sep 17 00:00:00 2001 From: MlssFrncJrg <119158464+MelissaFrncJrg@users.noreply.github.com> Date: Thu, 11 Jun 2026 10:33:12 +0200 Subject: [PATCH] feat(rest-api): add RBAC check for VM endpoints (#9910) * feat(rest-api): add RBAC check for VM endpoints --- @xen-orchestra/acl/src/actions/host.mts | 1 + @xen-orchestra/acl/src/actions/vm.mts | 2 ++ .../rest-api/src/vms/vm.controller.mts | 21 +++++++++++++++++-- CHANGELOG.unreleased.md | 2 ++ docs/docs/xo6/acl-v2.md | 4 ++-- 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/@xen-orchestra/acl/src/actions/host.mts b/@xen-orchestra/acl/src/actions/host.mts index 6cd3a6c411..d3bcdde614 100644 --- a/@xen-orchestra/acl/src/actions/host.mts +++ b/@xen-orchestra/acl/src/actions/host.mts @@ -1,6 +1,7 @@ export default { 'allow-vm': true, disable: true, + 'migrate-receive': true, enable: true, evacuate: true, export: { diff --git a/@xen-orchestra/acl/src/actions/vm.mts b/@xen-orchestra/acl/src/actions/vm.mts index 93ca1cd235..77399b259c 100644 --- a/@xen-orchestra/acl/src/actions/vm.mts +++ b/@xen-orchestra/acl/src/actions/vm.mts @@ -1,6 +1,8 @@ export default { + clone: true, delete: true, export: true, + 'migrate-send': true, pause: true, read: true, reboot: { diff --git a/@xen-orchestra/rest-api/src/vms/vm.controller.mts b/@xen-orchestra/rest-api/src/vms/vm.controller.mts index 6a77bab078..a245e195f1 100644 --- a/@xen-orchestra/rest-api/src/vms/vm.controller.mts +++ b/@xen-orchestra/rest-api/src/vms/vm.controller.mts @@ -747,6 +747,8 @@ export class VmController extends XapiXoController { } /** + * Required privilege: + * - resource: vm, action: clone * * - For fast clone on the same SR, omit `srId` and set `fast` to `true`. * - For full copy on the same SR, omit `srId` and set `fast` to `false`. @@ -758,8 +760,9 @@ export class VmController extends XapiXoController { @Example(taskLocation) @Extension('x-mcp-exposure', 'confirm') @Post('{id}/actions/clone') - @Middlewares(json()) + @Middlewares([json(), acl({ resource: 'vm', action: 'clone', objectId: 'params.id' })]) @SuccessResponse(asynchronousActionResp.status, asynchronousActionResp.description) + @Response(forbiddenOperationResp.status, forbiddenOperationResp.description) @Response(createdResp.status, createdResp.description) @Response(notFoundResp.status, notFoundResp.description) @Response(internalServerErrorResp.status, internalServerErrorResp.description) @@ -1061,6 +1064,10 @@ export class VmController extends XapiXoController { } /** + * Required privileges: + * - resource: vm, action: migrate-send + * - resource: host, action: migrate-receive (on the destination host) + * * VIF mapping is not allowed for intra-pool migration * * Networks and SRs must belong to the same pool as the destination host @@ -1071,8 +1078,17 @@ export class VmController extends XapiXoController { @Example(taskLocation) @Extension('x-mcp-exposure', 'confirm') @Post('{id}/actions/migrate') - @Middlewares(json()) + @Middlewares([ + json(), + // Two separate checks allow independent control so a user can be allowed to migrate a VM away + // without being allowed to place VMs on any specific host, and vice versa. + acl([ + { resource: 'vm', action: 'migrate-send', objectId: 'params.id' }, + { resource: 'host', action: 'migrate-receive', objectId: 'body.hostId' }, + ]), + ]) @SuccessResponse(asynchronousActionResp.status, asynchronousActionResp.description) + @Response(forbiddenOperationResp.status, forbiddenOperationResp.description) @Response(noContentResp.status, noContentResp.description) @Response(notFoundResp.status, notFoundResp.description) @Response(invalidParametersResp.status, invalidParametersResp.description) @@ -1105,6 +1121,7 @@ export class VmController extends XapiXoController { migrationNetworkId: migrationNetworkId as XoNetwork['id'] | undefined, sr: 'srId' in body ? (body.srId as XoSr['id']) : undefined, }) + return } return this.createAction(action, { diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index b97d81bc93..77e4a17344 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -37,6 +37,8 @@ - `DELETE /vdi-snapshots/{id}/tags/{tag}` (PR [#9906](https://github.com/vatesfr/xen-orchestra/pull/9906))/vdi-snapshots/{id}.{format}` (PR [#9906](https://github.com/vatesfr/xen-orchestra/pull/9906)) - `POST /vdis` (PR [#9908](https://github.com/vatesfr/xen-orchestra/pull/9908)) - `/vdis/:id/actions/migrate` (PR [#9908](https://github.com/vatesfr/xen-orchestra/pull/9908)) + - `/vms/:id/actions/clone` (PR [#9910](https://github.com/vatesfr/xen-orchestra/pull/9910)) + - `/vms/:id/actions/migrate` (PR [#9910](https://github.com/vatesfr/xen-orchestra/pull/9910)) - [XO6] live update XO tasks (PR [#9901](https://github.com/vatesfr/xen-orchestra/pull/9901)) - [XO6/Backup] add progress for backups tasks(PR [#9901](https://github.com/vatesfr/xen-orchestra/pull/9901)) diff --git a/docs/docs/xo6/acl-v2.md b/docs/docs/xo6/acl-v2.md index c68ddebc46..983c4f1f5a 100644 --- a/docs/docs/xo6/acl-v2.md +++ b/docs/docs/xo6/acl-v2.md @@ -106,7 +106,7 @@ Actions are written using the exact string you pass in a privilege. A parent act | Resource | Available actions | | --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `vm` | `read`, `delete`, `export`, `pause`, `start`, `resume`, `revert-snapshot`, `snapshot`, `suspend`, `unpause`, `reboot:clean`, `reboot:hard`, `shutdown:clean`, `shutdown:hard`, `update:affinityHost`, `update:autoPoweron`, `update:blockedOperations`, `update:coresPerSocket`, `update:cpuCap`, `update:cpuMask`, `update:cpuWeight`, `update:cpus`, `update:cpusStaticMax`, `update:creation`, `update:datasources`, `update:expNestedHvm`, `update:hasVendorDevice`, `update:highAvailability`, `update:hvmBootFirmware`, `update:memory`, `update:memoryMax`, `update:memoryMin`, `update:memoryStaticMax`, `update:nameDescription`, `update:nameLabel`, `update:nestedVirt`, `update:nicType`, `update:notes`, `update:PV_args`, `update:resourceSet`, `update:secureBoot`, `update:share`, `update:startDelay`, `update:suspendSr`, `update:tags`, `update:uefiMode`, `update:vga`, `update:videoram`, `update:viridian`, `update:virtualizationMode`, `update:xenStoreData` | +| `vm` | `read`, `clone`, `delete`, `migrate-send`, `export`, `pause`, `start`, `resume`, `revert-snapshot`, `snapshot`, `suspend`, `unpause`, `reboot:clean`, `reboot:hard`, `shutdown:clean`, `shutdown:hard`, `update:affinityHost`, `update:autoPoweron`, `update:blockedOperations`, `update:coresPerSocket`, `update:cpuCap`, `update:cpuMask`, `update:cpuWeight`, `update:cpus`, `update:cpusStaticMax`, `update:creation`, `update:datasources`, `update:expNestedHvm`, `update:hasVendorDevice`, `update:highAvailability`, `update:hvmBootFirmware`, `update:memory`, `update:memoryMax`, `update:memoryMin`, `update:memoryStaticMax`, `update:nameDescription`, `update:nameLabel`, `update:nestedVirt`, `update:nicType`, `update:notes`, `update:PV_args`, `update:resourceSet`, `update:secureBoot`, `update:share`, `update:startDelay`, `update:suspendSr`, `update:tags`, `update:uefiMode`, `update:vga`, `update:videoram`, `update:viridian`, `update:virtualizationMode`, `update:xenStoreData` | | `vm-snapshot` | `read`, `delete`, `export`, `update:tags` | | `vm-template` | `read`, `delete`, `export`, `instantiate`, `update:tags` | | `vm-controller` | `read`, `update:tags` | @@ -116,7 +116,7 @@ Actions are written using the exact string you pass in a privilege. A parent act | `vif` | `connect`, `create`, `delete`, `disconnect`,`read` | | `vbd` | `read`, `create`, `delete`, `connect`, `disconnect` | | `sr` | `read`, `delete`, `forget`, `migrate-receive`, `reclaim-space`, `scan`, `import:vdi`, `import:vm`, `update:tags` | -| `host` | `read`, `allow-vm`, `join-pool`, `export:logs`, `update:tags`, `disable`, `enable`, `evacuate` | +| `host` | `read`, `allow-vm`, `join-pool`, `export:logs`,`migrate-receive`, `update:tags`, `disable`, `enable`, `evacuate` | | `pool` | `add-host`, `read`, `emergency-shutdown`, `rolling-reboot`, `rolling-update`, `create:network`, `create:vm`, `update:tags` | | `network` | `read`, `create`, `delete`, `update:tags` | | `pif` | `read`, `update:management` |