check runtime delete action instead of workspace owner role

[dvoet]

Jira ticket: https://broadworkbench.atlassian.net/browse/IA-5025

Summary of changes

allow users with workspace delete action instead of workspace owner role to delete runtimes

Why

requiring owner role requires granting too much access to the rawls SA which needs to perform workspace delete

Testing these changes

What to test

Who tested and where

• [ ] This change is covered by automated tests
  • NB: Rerun automation tests on this PR by commenting jenkins retest or jenkins multi-test.
• [ ] I validated this change
• [ ] Primary reviewer validated this change
• [ ] I validated this change in the dev environment

[mlilynolting]

@dvoet this looks good! Have you confirmed that a user with Owner role on a workspace and no special access to runtimes under it can successfully delete any runtime? I'm not sure how access to the Delete action propagates.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.92%. Comparing base (80bdc97) to head (400e49e).

Additional details and impacted files

[](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere) ```diff @@ Coverage Diff @@ ## develop #4715 +/- ## =========================================== - Coverage 73.92% 73.92% -0.01% =========================================== Files 160 160 Lines 15000 15002 +2 Branches 1191 1205 +14 =========================================== + Hits 11089 11090 +1 - Misses 3911 3912 +1 ``` | [Files](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere) | Coverage Δ | | |---|---|---| | [...leonardo/http/service/RuntimeV2ServiceInterp.scala](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?src=pr&el=tree&filepath=http%2Fsrc%2Fmain%2Fscala%2Forg%2Fbroadinstitute%2Fdsde%2Fworkbench%2Fleonardo%2Fhttp%2Fservice%2FRuntimeV2ServiceInterp.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere#diff-aHR0cC9zcmMvbWFpbi9zY2FsYS9vcmcvYnJvYWRpbnN0aXR1dGUvZHNkZS93b3JrYmVuY2gvbGVvbmFyZG8vaHR0cC9zZXJ2aWNlL1J1bnRpbWVWMlNlcnZpY2VJbnRlcnAuc2NhbGE=) | `92.73% <100.00%> (+0.02%)` | :arrow_up: | | [...dinstitute/dsde/workbench/leonardo/samModels.scala](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fscala%2Forg%2Fbroadinstitute%2Fdsde%2Fworkbench%2Fleonardo%2FsamModels.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere#diff-Y29yZS9zcmMvbWFpbi9zY2FsYS9vcmcvYnJvYWRpbnN0aXR1dGUvZHNkZS93b3JrYmVuY2gvbGVvbmFyZG8vc2FtTW9kZWxzLnNjYWxh) | `0.00% <0.00%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere). Last update [80bdc97...400e49e](https://app.codecov.io/gh/DataBiosphere/leonardo/pull/4715?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataBiosphere).

[dvoet]

@dvoet this looks good! Have you confirmed that a user with Owner role on a workspace and no special access to runtimes under it can successfully delete any runtime? I'm not sure how access to the Delete action propagates.

Bummer, owners do not. I drilled into what the actual resource type is (the naming in Leo hides it) and it is controlled-application-private-workspace-resource. Only Leo has delete action on those. I am going to switch to checking delete on the workspace itself.

[mlilynolting]

Conceptually I don't like checking the delete-workspace action as a proxy for delete-workspace-resource. I don't think it's much of an improvement on just checking for Owner role.

[dvoet]

in my opinion, switching to an action check, even it if is not quite right, is vastly superior to a role check. Roles were never meant to gate access to features.

[mlilynolting]

in my opinion, switching to an action check, even it if is not quite right, is vastly superior to a role check. Roles were never meant to gate access to features.

That makes sense to me. Could you add a comment to the code indicating that a check on the runtime Delete action would be preferred but that it would require implementing a Sam hierarchy from Workspace to Runtimes?

[dvoet]

Could you add a comment to the code indicating that a check on the runtime Delete action would be preferred but that it would require implementing a Sam hierarchy from Workspace to Runtimes?

@mlilynolting I will cover this in my doc, but hierarchy is only part of the answer. Right now, there is no sam resource that exists that Leo can check to see if the caller should be able to delete the requested runtime. The right resource needs to exist and hierarchy can be used to make the correct permissions flow.

[mlilynolting]

That's a good point.