akuity / kargo

Application lifecycle orchestration
https://kargo.akuity.io/
Apache License 2.0
1.39k stars 114 forks source link

Manual Freight Approval Requires additional RBAC permissions than necessary #2228

Open ed-boykin opened 4 days ago

ed-boykin commented 4 days ago

Checklist

Description

While testing up a custom Role to allow a group to promote freight, discovered that additional permissions are required to allow Manual Approval and subsequent promotion.
For regular promotion, the stage/promote is needed to allow promotion For manual approval, the freights/patch is needed but also the freights/status/patch permission. Manual approval should only need freights/patch

Screenshots

Minimum permissions needed to promote and manually approve image

Steps to Reproduce

Create role with these freights verbs: Get, List, Watch, Patch Manually approve a freight. Manual approval denied.

Version

0.7.1


Paste the output from `kargo version` here.
Client Version: v0.7.1
krancour commented 3 days ago

@ed-boykin thanks for reporting this.

Couple things:

  1. By design, approving Freight for a given stage requires the promote verb on the Stage in question. We could not see the sense in allowing principals not authorized to promote to a given Stage to deem unverified Freight suitable for that Stage.

  2. For manual approval, the freights/patch is needed but also the freights/status/patch permission.

    This doesn't seem accurate. There is no need to patch Freight to execute a manual approval. A manual approval patches Freight status only.

Create role with these freights verbs: Get, List, Watch, Patch Manually approve a freight. Manual approval denied.

I would not have expected this to work, as it is missing permissions to patch Freight status.

The permission to patch Freight itself is extraneous in your example. Freight are mostly immutable, so the only reason to ever grant that would be to allow a user to edit Freight aliases.